|Anonymous | Login||04-11-2021 07:53 UTC|
|Main | My View | View Issues | Change Log | Docs|
|Viewing Issue Simple Details [ Jump to Notes ]||[ View Advanced ] [ Issue History ] [ Print ]|
|ID||Category||Severity||Reproducibility||Date Submitted||Last Update|
|0007017||[Squeak] Monticello||major||always||04-14-08 09:32||04-18-10 22:06|
|Summary||0007017: Sometimes Classes are not recompiled when instance variables change|
Under certain circumstances, instance variables do not get properly set up in a 3.10 image when code has been written in a 3.9 image, saved with Monticello to a repository and loaded with Monticello into 3.10.
This bug is quite a bit hard to reproduce. A scenario to reproduce it is to rename an instance variable in the class definition and then using this instance variable eg. in an accessor method. Even though this inst var gets properly set in the code using it, its value constantly points to a SmallInteger in 3.10, resulting in MNU when a message is sent to that inst var. In the debugger it then says SmallInteger does not understand this message, but inspecting the value of the var reveals that it's nil.
Here is a scenario with which I was able to reproduce the bug:
- Start with a fresh 3.9 dev-image, eg. sq3.9.1-7075dev08.04.1.
- Go the the class OBMonticelloPackageNode
- In the class definition, rename inst var 'workingCopy' to eg. 'workingCopy2', recompile the class
- Add a new 'dummy' inst var at the end of the inst var list, recompile again
- rename 'workingCopy2' back to 'workingCopy', recompile
- remove the dummy inst var, recompile
- Do not recompile any method using 'workingCopy'
- Save this code using Monticello to a repository
- Load the code from this repository into a fresh 3.10 dev image, eg. sq3.10-7159dev08.04.1
- Start the default browser, ie. an OB package browser.
- You should now get a MNU: SmallInteger>>package, 'workingCopy' is nil in that method, but the message #package gets sent to a SmallInteger. This is strange, and it's also strange that the inst var is nil, because there is a value getting set to it as you can see in the debugger. But this value simply does not get stored.
This is just one way to reproduce the error, there are others as well. It's important to always start with a fresh 3.10 image, the package with the changed code shouldn't have been loaded before by hand.
The very same code works well when loaded into a 3.9 image.
When the faulty class definition gets recompiled in 3.10, the problem disappears.
If you want to see a package generating that error, load OB-Enhancements-dr.145 from http://source.wiresong.ca/ob. [^]
Behavior-sourceMatchesBytecodeAt.st [^] (1,022 bytes) 05-22-08 19:42
MCIvarBugDemo-mtf.1.mcz [^] (912 bytes) 05-22-08 19:42
MCIvarBugDemo-mtf.3.mcz [^] (1,129 bytes) 05-22-08 19:42
SYSTEM WARNING: Creating default object from empty value
(0012024 - 78 - 78 - 78 - 78 - 78 - 78)
|Take care that 3.10 dev-images uses MC1.5 which is not the default MC of 3.10.|
(0012025 - 136 - 142 - 142 - 142 - 142 - 142)
The current workaround is to recompile the package after loading.
Later versions of MC may fix this, but really we need atomic loading.
(0012128 - 329 - 359 - 549 - 549 - 549 - 549)
I can confirm this bug: From the sq3.10-7159.08.04.1 image do the following:
Installer wiresong project: 'ob'; install: 'OB-Enhancements-dr.145'. OBPackageBrowser open.
This loads 'OB-Enhancements-dr.145' over 'OB-Enhancements-dr.138'.
The image is at:
(0012143 - 983 - 1081 - 1081 - 1081 - 1081 - 1081)
edited on: 05-22-08 20:05
Ok. Based on damien's example, I stripped the packages down to the bare minimum, and got the problem to reproduce itself when loading MCIvarBugDemo-mtf.3 (one class, one method) on top of MCIvarBugDemo-mtf.1 (one class, zero methods). I did not find an easy way to test this problem, so I also created Behavior >> #sourceMatchesBytecodeAt: in order to even detect the problem pragmatically. So, here is how to reproduce the problem, do the following in an image with MC1.5:
Installer mantis bug: 7017 fix: 'Behavior-sourceMatchesBytecodeAt.st'.
Installer mantis bug: 7017 fix: 'MCIvarBugDemo-mtf.1.mcz'.
Installer mantis bug: 7017 fix: 'MCIvarBugDemo-mtf.3.mcz'.
MCIvarBugDemo sourceMatchesBytecodeAt: #workingCopy. "-> false"
MCIvarBugDemo sourceMatchesBytecodeAt: #workingCopy. "-> true"
The test is repeatable: you can load version 1, then version 3 any number of times to recreate the problem. I'll turn it into a standalone test case later
(0012145 - 1732 - 1868 - 1868 - 1868 - 1868 - 1868)
edited on: 05-23-08 08:30
instance variables before: mcPackage theClass extensionClasses
instance variables after: workingCopy theClass extensionClasses
Expected result: the method will compile workingCopy to mean instance variable 1
Actual result: the method compiles workingCopy to mean instance variable 4
Execution trace. Set a few breakpoints in MCPackageLoader1b >> #basicLoad: to debug this:
class preload: during createUniClassWith:, the class is given four ivars: 'mcPackage theClass extensionClasses workingCopy'
method preload: compile the source code against the class. The ivar workingCopy is compiled as ivar 4. This method is not installed into the class.
class install: the class ivar shape is changed again, this time to its final form: 'workingCopy theClass extensionClasses. Note that workingCopy was moved from variable 4 to 1. The class still has no methods.
method install: The method, still with workingCopy bound to ivar 4, is added to the class without compilation
class postinstall: the class is again rebuilt with the correct ivars, however, since nothing changed, ClassBuilder optimizes away creating a new class and recompiling methods.
method postinstall: no action
method postload: nothing interesting
class postload: nothing interesting
So, there is a bug in the loader. I have no idea why MCClassDefinition >> #createUnionClassWith: exists, but it does the wrong thing. To be pseudo-atomic, one would NOT modify the class during preload, but instead, create a temporary class with the correct ivar layout, and compile methods against that (then changing the method's class pointer to the real class)
Or, as a workaround, put "aClass compileAll" in either the postInstall or postLoad of MCClassDefinition
(0012256 - 346 - 346 - 346 - 346 - 346 - 346)
|This bug is pretty tied-up with the pseudo-atomic loader that is MCPackageLoader1b. I don't see a way to fix this bug without making a completely new package loader. Fortunately, we have a new one in progress, which is based on SystemEditor, and should solve this bug, as well as make package loading truly atomic. I'm working on MCPackageLoader2|
(0012266 - 162 - 174 - 174 - 174 - 174 - 174)
Fixed in Monticello.impl-kph.515
There are two possible fixes a conservative one and a sledgehammer one, the conservative one has been used for the time being.
(0012272 - 30 - 30 - 30 - 30 - 30 - 30)
|David, can you confirm please?|
(0012478 - 161 - 161 - 161 - 161 - 161 - 161)
|I have now checked this fix and as far as I can tell it indeed fixes the problem. :) I couldn't reproduce the issue anymore, so I think we can close this report.|
|04-14-08 09:32||dr||New Issue|
|04-14-08 09:32||dr||Status||new => assigned|
|04-14-08 09:32||dr||Assigned To||=> avi|
|04-14-08 13:09||Damien Cassou||Note Added: 0012024|
|04-14-08 13:10||Damien Cassou||Assigned To||avi => Keith_Hodges|
|04-14-08 13:35||Keith_Hodges||Note Added: 0012025|
|05-05-08 11:57||Damien Cassou||Relationship added||has duplicate 0007034|
|05-20-08 22:33||matthewf||Note Added: 0012128|
|05-22-08 19:40||matthewf||Issue Monitored: matthewf|
|05-22-08 19:41||matthewf||Summary||Loading code written in 3.9 gets not loaded properly into 3.10 when instance variables have been renamed => Sometimes Classes are not recompiled when instance variables change|
|05-22-08 19:42||matthewf||File Added: Behavior-sourceMatchesBytecodeAt.st|
|05-22-08 19:42||matthewf||File Added: MCIvarBugDemo-mtf.1.mcz|
|05-22-08 19:42||matthewf||File Added: MCIvarBugDemo-mtf.3.mcz|
|05-22-08 19:55||matthewf||Note Added: 0012143|
|05-22-08 19:59||matthewf||Note Edited: 0012143|
|05-22-08 20:01||matthewf||Note Edited: 0012143|
|05-22-08 20:05||matthewf||Note Edited: 0012143|
|05-23-08 08:16||matthewf||Note Added: 0012145|
|05-23-08 08:30||matthewf||Note Edited: 0012145|
|06-04-08 23:36||matthewf||Note Added: 0012256|
|06-07-08 02:25||Keith_Hodges||Note Added: 0012266|
|06-07-08 02:36||Keith_Hodges||Status||assigned => feedback|
|06-07-08 02:58||matthewf||Relationship added||related to 0006241|
|06-07-08 17:40||Damien Cassou||Note Added: 0012272|
|08-11-08 08:55||dr||Note Added: 0012478|
|10-07-08 12:52||Keith_Hodges||Status||feedback => resolved|
|10-07-08 12:52||Keith_Hodges||Resolution||open => fixed|
|04-18-10 22:06||andreas||Status||resolved => closed|
| Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
105 total queries executed.|
55 unique queries executed.