Mantis Bugtracker
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
Reporter dr View Status public  
Assigned To Keith_Hodges
Priority normal Resolution fixed  
Status closed   Product Version 3.10
Summary 0007017: Sometimes Classes are not recompiled when instance variables change
Description 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.
Additional Information 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. [^]
Attached Files  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

- Relationships

SYSTEM WARNING: Creating default object from empty value

has duplicate 0007034closed Damien Cassou Squeak Packages upgrading to OB Enhancements version 0.145 breaks OB 
related to 0006241assigned andreas Squeak Atomic loading 

- Notes
(0012024 - 78 - 78 - 78 - 78 - 78 - 78)
Damien Cassou
04-14-08 13:09

 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)
Keith_Hodges
04-14-08 13:35

 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)
matthewf
05-20-08 22:33

 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:
http://gforge.inria.fr/frs/download.php/4379/sq3.10-7159dev08.04.1.zip [^]
 
(0012143 - 983 - 1081 - 1081 - 1081 - 1081 - 1081)
matthewf
05-22-08 19:55
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 compileAll.
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)
matthewf
05-23-08 08:16
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)
matthewf
06-04-08 23:36

 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)
Keith_Hodges
06-07-08 02:25

 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)
Damien Cassou
06-07-08 17:40

 David, can you confirm please?
 
(0012478 - 161 - 161 - 161 - 161 - 161 - 161)
dr
08-11-08 08:55

 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.
 

- Issue History
Date Modified Username Field Change
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.
Powered by Mantis Bugtracker