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

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


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