[squeak-dev] [BUG][FIX] Bug in WeakMessageSend

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Tue May 12 20:45:03 UTC 2009


Oh, and I forgot, correct class comment:

 shouldBeNil		Array of: Boolean --  used to distinguish nil arguments
from garbage collected arguments

2009/5/12 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
> Oops I did not understand the API at first glance...
> 3) was wrong because arguments are added first, and anArray last -
>
> One more thing: WeakActionSequence using isValid test is fragile too...
>
>
> 2009/5/12 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
>> Yes sure, that was stupid!
>>
>> But:
>>
>> 1) #value & al used to return the value of evaluating the message, or
>> nil if any receiver/argument garbaged.
>> Now #value & al will always return self
>>
>> 2) #valueWithArguments: should not care if arguments were garbage
>> collected, because some replacement arguments were provided
>>
>> 3) #valueWithEnoughArguments: should care of garbage collected
>> arguments from: anArray size to: arguments size.
>>
>> Please publish a mantis report, then upload your updated code :)
>>
>> Cheers
>>
>> Nicolas
>>
>> 2009/5/12 Juan Vuletich <juan at jvuletich.org>:
>>> Hi Folks,
>>>
>>> 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.
>>>
>>> The problem happens in Squeak 3.10, Pharo-10243, and Cuis.
>>>
>>> Please take a look. Also take a look at my proposed solution. I'm not sure,
>>> maybe some will object keeping an artificial reference to an object that
>>> could have been collected otherwise, eve if it is just for the time of a
>>> message send... Anyway, comments welcome.
>>>
>>> Cheers,
>>> Juan Vuletich
>>>
>>> 'From Squeak3.7 of ''4 September 2004'' [latest update: #5989] on 12 May
>>> 2009 at 11:02:25 am'!
>>>
>>> !WeakMessageSend methodsFor: 'evaluating' stamp: 'jmv 5/12/2009 10:59'!
>>> value
>>>        ^ arguments isNil
>>>                ifTrue: [
>>>                        self withEnsuredReceiverDo: [ :r | r perform:
>>> selector]]
>>>                ifFalse: [
>>>                        self withEnsuredReceiverAndArgumentsDo: [ :r :a |
>>>                                r
>>>                                        perform: selector
>>>                                        withArguments: a]]! !
>>>
>>> !WeakMessageSend methodsFor: 'evaluating' stamp: 'jmv 5/12/2009 11:00'!
>>> valueWithArguments: anArray
>>>        | argsToUse |
>>>
>>>        "Safe to use, because they are built before ensureing receiver and
>>> args..."
>>>        argsToUse _ self collectArguments: anArray.
>>>        ^self withEnsuredReceiverAndArgumentsDo: [ :r :a |
>>>                                r
>>>                                        perform: selector
>>>                                        withArguments: argsToUse ]! !
>>>
>>> !WeakMessageSend methodsFor: 'evaluating' stamp: 'jmv 5/12/2009 11:01'!
>>> valueWithEnoughArguments: anArray
>>>
>>>        "call the selector with enough arguments from arguments and anArray"
>>>        | args |
>>>        args _ Array new: selector numArgs.
>>>        args replaceFrom: 1
>>>                to: ( arguments size min: args size)
>>>                with: arguments
>>>                startingAt: 1.
>>>        args size > arguments size ifTrue: [
>>>                args replaceFrom: arguments size + 1
>>>                        to: (arguments size + anArray size min: args size)
>>>                        with: anArray
>>>                        startingAt: 1.
>>>        ].
>>>
>>>        "args is safe to use, because they are built before ensureing
>>> receiver and args..."
>>>        ^self withEnsuredReceiverAndArgumentsDo: [ :r :a |
>>>                                r
>>>                                        perform: selector
>>>                                        withArguments: args ]! !
>>>
>>> !WeakMessageSend methodsFor: 'private' stamp: 'jmv 5/12/2009 10:54'!
>>> withEnsuredReceiverAndArgumentsDo: aBlock
>>>        "Grab real references to receiver and arguments. If they still exist,
>>> evaluate aBlock."
>>>
>>>        "Return if my receiver has gone away"
>>>        | r a |
>>>        r _ self receiver.
>>>        r ifNil: [ ^self ].
>>>
>>>
>>>        "Make sure that my arguments haven't gone away"
>>>        "arguments can not become nil. The extra care in
>>> #ensureReceiverAndArguments is wrong
>>>        (BTW, the whole idea is wrong, that's why we do these new
>>> methods...)"
>>>        a _ Array withAll: arguments.
>>>        a with: shouldBeNil do: [ :arg :flag |
>>>                arg ifNil: [ flag ifFalse: [ ^self ]]
>>>        ].
>>>
>>>        aBlock value: r value: a! !
>>>
>>> !WeakMessageSend methodsFor: 'private' stamp: 'jmv 5/12/2009 10:53'!
>>> withEnsuredReceiverDo: aBlock
>>>        "Grab a real reference to receive. If still there, evaluate aBlock."
>>>
>>>        "Return if my receiver has gone away"
>>>        | r |
>>>        r _ self receiver.
>>>        r ifNil: [ ^self ].
>>>
>>>        aBlock value: r! !
>>>
>>> WeakMessageSend removeSelector: #ensureReceiver!
>>> WeakMessageSend removeSelector: #ensureReceiverAndArguments!
>>>
>>>
>>>
>>>
>>
>



More information about the Squeak-dev mailing list