Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0007352 [Squeak] Kernel major random 05-13-09 13:53 02-06-11 23:48
Reporter jmvuletich View Status public  
Assigned To nicolas cellier
Priority normal Resolution fixed  
Status closed   Product Version 3.10.2bc
Summary 0007352: WeakMessageSend generates a DNU because of the message receiver being collected and disappearing
Description I found a bug in WeakMessageSend that generates a DNU because of the message receiver being collected and disappearing. The culprits are #ensureReceiver and #ensureReceiverAndArguments . These methods ensure nothing. It might happen (although not often) that the receiver or some argument is collected after this method is called, but before it is actually used.
Additional Information The problem happens in Squeak 3.10, Pharo-10243, and Cuis.
Attached Files  WeakMessageSendFixPart1-jmv-M7352.cs [^] (865 bytes) 05-13-09 14:43
 WeakMessageSendFixPart2-jmv-M7352.cs [^] (5,502 bytes) 05-13-09 14:44
 WeakMessageSendFixPart1-jmv-M7352.9.cs [^] (1,055 bytes) 05-14-09 15:11
 WeakMessageSendFixPart2-jmv-M7352.8.cs [^] (6,380 bytes) 05-14-09 15:12

- Relationships

- Notes
(0013126 - 1521 - 1635 - 1635 - 1635 - 1635 - 1635)
nicolas cellier
05-13-09 22:11

Hi Juan, I approve the essential of the fix, however there are some details to be reviewed, so here are my code critics.
Changing this piece of art is not that straight forward!

1) #isReceiverOrAnyArgumentGarbage is a message only in WeakMessageSend
#isValid was also understood by MessageSend.

Now, with new definition of WeakActionSequence>>#asMinimalRepresentation, a WeakActionSequence cannot include a mixture of MessageSend and WeakMessageSend.
In old time, it could have. Is it on purpose?

2) Catching Exception (WeakActionSequenceTrappingError) is considered bad and dangerous, so watch your steps. You should maybe consider catching Error instead.

3) You have broken WeakActionSequence>>#value, because it will now return nil when last action is garbaged. You can't rely on using #asMinimalRepresentation previously because garbage collection can occur in between as you noticed. I don't know if this has any implication, but the API is clearly changed.

4) Beware, #isValid has another definition in WeakMessageSend subclass: #NonReentrantWeakMessageSend.
With your new usage of #asMinimalRepresentation, it would have been wrong to use NonReentrantWeakMessageSend.>>#isValid because it would exclude definitely a temporarily unavailable action... Unfortunately that also excludes MessageSend - see 1)

Beware if you correct 3), then a NonReentrantWeakMessageSend that is not valid but is not garbaged will return nil when evaluated, and this will be assigned to answer when it should not...
 
(0013127 - 582 - 634 - 634 - 634 - 634 - 634)
jmvuletich
05-14-09 15:17

Hi Nicolas, thanks for reviewing!
1) Fixed. Added #isReceiverOrAnyArgumentGarbage to MessageSend. Did not add back #isValid because it is "false polymorphism", a message implemented by objects that are not polymorphic at all. This is bad in general, it adds confusion and fills 'implementors' and 'senders' with irrelevant stuff.

2) That problem was already there before. I fixed it.

3) Fixed

4) WeakMessageSend does not have any subclass in stock 3.10. It seems that those are part of the PolyMorph package. I guess its mainainers need to update this. I did not fix this.
 
(0013142 - 596 - 638 - 638 - 638 - 638 - 638)
GazzaGuru
07-02-09 12:42

In terms of the semantics of NonReentrantWeakMessageSend, #isValid is not really necessary... the rentrancy is protected upon the call to #valueWithArguments:.

I'll remove in next update.

As for #valueWithEnoughArguments: it *could* be sent in our 3.9 images, just happens not to be used (only used with MessageSend in that image).
I can remove those from Polymorph too, with the caveat that they shouldn't be called in older images.

Of course, when a block is used as an action there's a problem (defines #isValid but not #isReceiverOrAnyArgumentGarbage). Then there's BlockClosures...
 
(0013843 - 314 - 356 - 806 - 806 - 806 - 806)
nicolas cellier
08-22-10 19:43

Fixed in:
http://source.squeak.org/trunk/Kernel-nice.482.mcz [^]
http://source.squeak.org/trunk/System-nice.360.mcz [^]
http://source.squeak.org/trunk/Kernel-nice.483.mcz [^]

Note I did not remove WeakMessageSend>>valueWithEnoughArguments: though it is not used with such receiver.
Who knows, a package might need it...
 

- Issue History
Date Modified Username Field Change
05-13-09 13:53 jmvuletich New Issue
05-13-09 13:55 jmvuletich File Added: 7352-WeakMessageSendFixPart1-jmv.5.cs
05-13-09 13:55 jmvuletich File Added: 7352-WeakMessageSendFixPart2-jmv.4.cs
05-13-09 14:05 jmvuletich Severity minor => major
05-13-09 14:43 jmvuletich File Deleted: 7352-WeakMessageSendFixPart1-jmv.5.cs
05-13-09 14:43 jmvuletich File Deleted: 7352-WeakMessageSendFixPart2-jmv.4.cs
05-13-09 14:43 jmvuletich File Added: WeakMessageSendFixPart1-jmv-M7352.cs
05-13-09 14:44 jmvuletich File Added: WeakMessageSendFixPart2-jmv-M7352.cs
05-13-09 22:11 nicolas cellier Note Added: 0013126
05-14-09 15:11 jmvuletich File Added: WeakMessageSendFixPart1-jmv-M7352.9.cs
05-14-09 15:12 jmvuletich File Added: WeakMessageSendFixPart2-jmv-M7352.8.cs
05-14-09 15:17 jmvuletich Note Added: 0013127
07-02-09 12:42 GazzaGuru Note Added: 0013142
08-22-10 19:43 nicolas cellier Status new => resolved
08-22-10 19:43 nicolas cellier Fixed in Version  => trunk
08-22-10 19:43 nicolas cellier Resolution open => fixed
08-22-10 19:43 nicolas cellier Assigned To  => nicolas cellier
08-22-10 19:43 nicolas cellier Note Added: 0013843
02-06-11 23:48 leves Status resolved => closed


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
68 total queries executed.
40 unique queries executed.
Powered by Mantis Bugtracker