(integrating ClassRenameFixTest was: Re: Collecting actions for RC2)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

(integrating ClassRenameFixTest was: Re: Collecting actions for RC2)

Chris Muller
Hi Roel,

> 1- I am investigating your fix, and it looks ok. One question though:...

Yeah, because of the need for the oldName.  Before the fix, the system would not set the class's (new) name until after renameClass:as: was called, and that method did all the clean up (such as removing the class-name key from Smalltalk) based on the (still old) class name.  At the bottom of renameClass:as: it then signals the SystemChangeNotifier.  Everything is "cleaned up" except, OOPS, the class' name itself is still the old name.  This is the inconsistent state that.

To fix the problem, I set the class' name *before* calling the new method, renameClass:from:, and passing in the old name from which we can do all the Smalltalk cleanups before finally signaling the SystemChangeNotifier while all clean and consistent.

Hope that made sense..

I don't like the mostly-duplicated code either, but its more important to me for it to work correctly and then we can figure out how to get rid of renameClass:as: later..

> 2- I am a bit worried about your remark about the obsolete class and
> the bad event handler...
> Do you still have an image with this problem ?

3.9-7061 has the problem.  I think its just a event-handler that didn't get cleaned up from one of the tests due to unusual circumstances.  3.9 underwent a lot of changes, bringing in Traits, new Compiler.  Who knows what all Stef did in the wee hours, maybe exiting with debugger open and then ensure's didn't get called or something, who knows..  I don't think its anything to worry about; the important things are:

  - we can (and should) easily clean it up in the existing 7061 image
  - whatever caused it was probably a very unusual situation related to the construction of the 3.9 image
  - the test cleans up after itself, we can't get the problem to happen again
  - the problem is an artifact of only the test, even if we didn't clean it, it doesn't affect anything else, not even the bug fix

Thanks,
  Chris




Reply | Threaded
Open this post in threaded view
|

Re: (integrating ClassRenameFixTest was: Re: Collecting actions for RC2)

stephane ducasse-2
HI chris

I'm going over RC2. What is the status for this problem?

> Yeah, because of the need for the oldName.  Before the fix, the  
> system would not set the class's (new) name until after  
> renameClass:as: was called, and that method did all the clean up  
> (such as removing the class-name key from Smalltalk) based on the  
> (still old) class name.  At the bottom of renameClass:as: it then  
> signals the SystemChangeNotifier.  Everything is "cleaned up"  
> except, OOPS, the class' name itself is still the old name.  This  
> is the inconsistent state that.
>
> To fix the problem, I set the class' name *before* calling the new  
> method, renameClass:from:, and passing in the old name from which  
> we can do all the Smalltalk cleanups before finally signaling the  
> SystemChangeNotifier while all clean and consistent.
>
> Hope that made sense..
>
> I don't like the mostly-duplicated code either, but its more  
> important to me for it to work correctly and then we can figure out  
> how to get rid of renameClass:as: later..
>
>> 2- I am a bit worried about your remark about the obsolete class and
>> the bad event handler...
>> Do you still have an image with this problem ?
>
> 3.9-7061 has the problem.  I think its just a event-handler that  
> didn't get cleaned up from one of the tests due to unusual  
> circumstances.  3.9 underwent a lot of changes, bringing in Traits,  
> new Compiler.  Who knows what all Stef did in the wee hours, maybe  
> exiting with debugger open and then ensure's didn't get called or  
> something, who knows..  I don't think its anything to worry about;  
> the important things are:
>
>   - we can (and should) easily clean it up in the existing 7061 image
>   - whatever caused it was probably a very unusual situation  
> related to the construction of the 3.9 image
>   - the test cleans up after itself, we can't get the problem to  
> happen again
>   - the problem is an artifact of only the test, even if we didn't  
> clean it, it doesn't affect anything else, not even the bug fix
>
> Thanks,
>   Chris
>
>
>
>