Mantis - Squeak
Viewing Issue Advanced Details
209 VM crash always 10-03-04 09:58 12-03-04 00:41
craig  
tim  
normal  
closed  
fixed  
none    
none 3.8  
0000209: (object become: object) kills the VM
Attempt to make an object become itself. The virtual machine crashes.
 BecomeSelfImagefix.1.cs [^] (1,640 bytes) 11-24-04 17:16
 BecomeSelfVMfix.2.cs [^] (4,450 bytes) 11-30-04 03:43

Notes
(0000561)
tim   
10-30-04 05:18   
Hmm, interesting one. I _think_ the best place to fix this is in the #prepareForwardingTableForBecoming.... method since there is where we get to run down the two become-arrays. If we simply skip any pair where oop1 == oop2 all ought to be ok.

Comments?
(0000600)
johnmci   
11-06-04 04:56   
Cross checked and compiled on os-x. Confirmed it fixes foo become: foo. Check for object identity is done as part of logic that checks for SmallInteger etc. However requires image change, if image change is not in image then you get recursive method call which causes failure (maybe). However this is much better than crashing the VM
(0000737)
tim   
11-24-04 17:15   
reopen to attach fix file
(0000738)
tim   
11-24-04 17:17   
re-resolve now the file is attached...
(0000739)
tim   
11-24-04 17:19   
sigh, one more time, this mantis thig is _so_ cumbersome...
(0000754)
MarcusDenker   
11-25-04 14:02   
added
(0000775)
tim   
11-30-04 03:40   
Reopen to add new fix and information.
(0000776)
tim   
11-30-04 03:45   
Hokay, after some head-scratching, debugging and experimenting I have
an explanation of why 'a become a' will blow the VM, why 'a
forwardBecome a' is ok and a plausible fix.

The key problem is that building the forwarding blocks used (along with
the compaction phase of the garbage collector) is not idempotent - the
first time it is done for an object _corrupts_ that object and stores
restoration info in the forwarding block. That's how the compaction
works. However, refer to the same object in a become so that you need
to build another forwarding block and you're violatin' th'law, boy. You
end up with the poor object owning a forwarding block with a completely
evil set of bits. When it comes to the compacting code's turn to do its
job the excrement impacts the rotating impeller since insted of a nice
object header word it stumbles over a mangled pointer with extra fries,
err, flags.

So now we can see why 'a become 'a fails - two forwarding blocks for one
object, and why 'a forwardBecome a' is ok - only one block is built
per forwared object. Note that we will also get into trouble if any
attempt is made to do
{a.b} become {b.a}
because of two forwarding blocks for both a & b. Likewise, expect
problems with
{a. a} becomeForward {b. c}.
I doubt anyone would deliberately write either case but code using the
become facility might easily end up doing so; for example ImageSegment
loading seems to often do 'a becomeForward a' type mutations. If it
happens that an object is refered to twice in that process, ka-boom.

The initial VM fix for the failure of 'a become a' was to fail the prim
but that stopped SqueakMap from loading because of the ImageSegment
issue mentioned above. My revised suggestion works somewhat better and
simply skips the a=>a pairs in the become prim loop. This means that
there is some time 'wasted' in such cases but at least it is cpu time
and not frustrated user time. The fix DOES NOT prevent problems such as
{a. a} becomeForward {b. c}.
which I strongly think would be better caught upfront in the image. In
general I don't see any meaningful reason to allow things like
{a.b} become {b.a}
(0000818)
tim   
12-03-04 00:40   
The changes are included in the VMMaker38b1 package.
We now have a vm where
a becomeForward a works ok by doing nothing (duh)
a become a works by doing nothing
{a.a} become* {*.*} will probably blow the vm and it's your problem.

It's not perfect but it is better.
(0000819)
tim   
12-03-04 00:41   
Closed for 3.8