Pharo 1.0 & 1.1 failing tests testFinalizationOfEquals

> I wrote this test pre-closure.
> The problem with closures is that they are optimized to retain a
> pointer to the values of outer temporary variable they use when
> activated.
> Thus the BlockClosures forAnyTwoEqualObjects ofDifferentIdentity
> registeringAnActionAtFinalizationForEachObject all refer to 'hello'
> pointed by o1 and its copy pointed by o2.
> In other words, o1 and o2 are not the single pointers to the String
> 'hello' and its copy.
> thenForcingFinalizationOfObjects only clean o1 and o2, but omit to
> clean the closures that still point to 'hello'...
> Thus, 'hello' is not finalized.
> One possibility is to also clean the closures pointing to 'hello'.
> But now the code is tricky... It's aim was to be simple damned :)
>
> Nicolas
>
>
> here is a corrected version :
>
>
> testFinalizationOfEquals
>        "self debug: #testFinalizationOfEquals"
>
>        | finalizationProbe o1 o2
>        forAnyTwoEqualObjects
>        ofDifferentIdentity
>        registeringAnActionAtFinalizationForEachObject
>        thenForcingFinalizationOfObjects
>        implyBothRegisteredActionsAreExecuted |
>
>        finalizationProbe := Set new.
>        o1 := 'hello' copy.
>        o2 := 'hello' copy.
>        forAnyTwoEqualObjects := [o1 = o2].
>        ofDifferentIdentity := [o1 ~~ o2].
>        registeringAnActionAtFinalizationForEachObject := [
>                o1 toFinalizeSend: #add: to: finalizationProbe  with: 'first object finalized'.
>                o2 toFinalizeSend: #add: to: finalizationProbe  with: 'second object
> finalized'].
>        thenForcingFinalizationOfObjects := [
>                forAnyTwoEqualObjects := ofDifferentIdentity :=
> registeringAnActionAtFinalizationForEachObject := nil.
>                o1 := o2 := nil. Smalltalk garbageCollect].
>        implyBothRegisteredActionsAreExecuted := [finalizationProbe size = 2].
>
>        self
>                assert: forAnyTwoEqualObjects;
>                assert: ofDifferentIdentity;
>                should: [
>                        registeringAnActionAtFinalizationForEachObject value.
>                        thenForcingFinalizationOfObjects value.
>                        implyBothRegisteredActionsAreExecuted value].
>

> Oh, now it fails due to WeakIdentityKeyDictionary bug...
>
> Nicolas
>
> 2010/1/16 Nicolas Cellier <[hidden email]>:
>> I wrote this test pre-closure.
>> The problem with closures is that they are optimized to retain a
>> pointer to the values of outer temporary variable they use when
>> activated.
>> Thus the BlockClosures forAnyTwoEqualObjects ofDifferentIdentity
>> registeringAnActionAtFinalizationForEachObject all refer to 'hello'
>> pointed by o1 and its copy pointed by o2.
>> In other words, o1 and o2 are not the single pointers to the String
>> 'hello' and its copy.
>> thenForcingFinalizationOfObjects only clean o1 and o2, but omit to
>> clean the closures that still point to 'hello'...
>> Thus, 'hello' is not finalized.
>> One possibility is to also clean the closures pointing to 'hello'.
>> But now the code is tricky... It's aim was to be simple damned :)
>>
>> Nicolas
>>
>>
>> here is a corrected version :
>>
>>
>> testFinalizationOfEquals
>>        "self debug: #testFinalizationOfEquals"
>>
>>        | finalizationProbe o1 o2
>>        forAnyTwoEqualObjects
>>        ofDifferentIdentity
>>        registeringAnActionAtFinalizationForEachObject
>>        thenForcingFinalizationOfObjects
>>        implyBothRegisteredActionsAreExecuted |
>>
>>        finalizationProbe := Set new.
>>        o1 := 'hello' copy.
>>        o2 := 'hello' copy.
>>        forAnyTwoEqualObjects := [o1 = o2].
>>        ofDifferentIdentity := [o1 ~~ o2].
>>        registeringAnActionAtFinalizationForEachObject := [
>>                o1 toFinalizeSend: #add: to: finalizationProbe  with: 'first object finalized'.
>>                o2 toFinalizeSend: #add: to: finalizationProbe  with: 'second object
>> finalized'].
>>        thenForcingFinalizationOfObjects := [
>>                forAnyTwoEqualObjects := ofDifferentIdentity :=
>> registeringAnActionAtFinalizationForEachObject := nil.
>>                o1 := o2 := nil. Smalltalk garbageCollect].
>>        implyBothRegisteredActionsAreExecuted := [finalizationProbe size = 2].
>>
>>        self
>>                assert: forAnyTwoEqualObjects;
>>                assert: ofDifferentIdentity;
>>                should: [
>>                        registeringAnActionAtFinalizationForEachObject value.
>>                        thenForcingFinalizationOfObjects value.
>>                        implyBothRegisteredActionsAreExecuted value].
>>
>

> Ah Ah, I found the guilty:
>
> Set>> do:  use 1 to: array size do: [:i | (array at: i) ...
>
> But array inst. var can be overwritten inside the loop due to #rehash
>
> OK, OK, I thought I would catch it be redefining do: in WeakKeyDictionary
>
> do: aBlock
>        "This needs to be redefined because array inst. var. can eventually
> be overwritten during the loop (due to rehash, due to fullCheck) and
> make super fail."
>
>        tally = 0 ifTrue: [^ self].
>        array do:
>                [:each |
>                each ifNotNil: [aBlock value: each]]
>

> But ARGHH SUPER EVIL, this method does not get executed BECAUSE :
>
> Dictionary>>associationsDo: aBlock
>        "Evaluate aBlock for each of the receiver's elements (key/value
>        associations)."
>
>        super do: aBlock
>
> ARGHH, it really uses super do:, not my brand new #do:
>
> This is a good reason I DON'T LIKE INVOKING super WITH A DIFFERENT selector...
>
> Does not matter, I will just redefine Set>>do:, but that was my evil
> minute of the day...
>
> Nicolas
>
> 2010/1/16 Nicolas Cellier <[hidden email]>:
>> Oh, now it fails due to WeakIdentityKeyDictionary bug...
>>
>> Nicolas
>>
>> 2010/1/16 Nicolas Cellier <[hidden email]>:
>>> I wrote this test pre-closure.
>>> The problem with closures is that they are optimized to retain a
>>> pointer to the values of outer temporary variable they use when
>>> activated.
>>> Thus the BlockClosures forAnyTwoEqualObjects ofDifferentIdentity
>>> registeringAnActionAtFinalizationForEachObject all refer to 'hello'
>>> pointed by o1 and its copy pointed by o2.
>>> In other words, o1 and o2 are not the single pointers to the String
>>> 'hello' and its copy.
>>> thenForcingFinalizationOfObjects only clean o1 and o2, but omit to
>>> clean the closures that still point to 'hello'...
>>> Thus, 'hello' is not finalized.
>>> One possibility is to also clean the closures pointing to 'hello'.
>>> But now the code is tricky... It's aim was to be simple damned :)
>>>
>>> Nicolas
>>>
>>>
>>> here is a corrected version :
>>>
>>>
>>> testFinalizationOfEquals
>>>        "self debug: #testFinalizationOfEquals"
>>>
>>>        | finalizationProbe o1 o2
>>>        forAnyTwoEqualObjects
>>>        ofDifferentIdentity
>>>        registeringAnActionAtFinalizationForEachObject
>>>        thenForcingFinalizationOfObjects
>>>        implyBothRegisteredActionsAreExecuted |
>>>
>>>        finalizationProbe := Set new.
>>>        o1 := 'hello' copy.
>>>        o2 := 'hello' copy.
>>>        forAnyTwoEqualObjects := [o1 = o2].
>>>        ofDifferentIdentity := [o1 ~~ o2].
>>>        registeringAnActionAtFinalizationForEachObject := [
>>>                o1 toFinalizeSend: #add: to: finalizationProbe  with: 'first object finalized'.
>>>                o2 toFinalizeSend: #add: to: finalizationProbe  with: 'second object
>>> finalized'].
>>>        thenForcingFinalizationOfObjects := [
>>>                forAnyTwoEqualObjects := ofDifferentIdentity :=
>>> registeringAnActionAtFinalizationForEachObject := nil.
>>>                o1 := o2 := nil. Smalltalk garbageCollect].
>>>        implyBothRegisteredActionsAreExecuted := [finalizationProbe size = 2].
>>>
>>>        self
>>>                assert: forAnyTwoEqualObjects;
>>>                assert: ofDifferentIdentity;
>>>                should: [
>>>                        registeringAnActionAtFinalizationForEachObject value.
>>>                        thenForcingFinalizationOfObjects value.
>>>                        implyBothRegisteredActionsAreExecuted value].
>>>
>>
>

> 2010/1/16 Nicolas Cellier <[hidden email]>:
>> Ah Ah, I found the guilty:
>>
>> Set>> do:  use 1 to: array size do: [:i | (array at: i) ...
>>
>> But array inst. var can be overwritten inside the loop due to #rehash
>>
>> OK, OK, I thought I would catch it be redefining do: in WeakKeyDictionary
>>
>> do: aBlock
>>        "This needs to be redefined because array inst. var. can eventually
>> be overwritten during the loop (due to rehash, due to fullCheck) and
>> make super fail."
>>
>>        tally = 0 ifTrue: [^ self].
>>        array do:
>>                [:each |
>>                each ifNotNil: [aBlock value: each]]
>>
>
> Never mind, this was a bad idea anyway, Dictionary do: should only
> iterate on values
>
>
>> But ARGHH SUPER EVIL, this method does not get executed BECAUSE :
>>
>> Dictionary>>associationsDo: aBlock
>>        "Evaluate aBlock for each of the receiver's elements (key/value
>>        associations)."
>>
>>        super do: aBlock
>>
>> ARGHH, it really uses super do:, not my brand new #do:
>>
>> This is a good reason I DON'T LIKE INVOKING super WITH A DIFFERENT selector...
>>
>> Does not matter, I will just redefine Set>>do:, but that was my evil
>> minute of the day...
>>
>> Nicolas
>>
>> 2010/1/16 Nicolas Cellier <[hidden email]>:
>>> Oh, now it fails due to WeakIdentityKeyDictionary bug...
>>>
>>> Nicolas
>>>
>>> 2010/1/16 Nicolas Cellier <[hidden email]>:
>>>> I wrote this test pre-closure.
>>>> The problem with closures is that they are optimized to retain a
>>>> pointer to the values of outer temporary variable they use when
>>>> activated.
>>>> Thus the BlockClosures forAnyTwoEqualObjects ofDifferentIdentity
>>>> registeringAnActionAtFinalizationForEachObject all refer to 'hello'
>>>> pointed by o1 and its copy pointed by o2.
>>>> In other words, o1 and o2 are not the single pointers to the String
>>>> 'hello' and its copy.
>>>> thenForcingFinalizationOfObjects only clean o1 and o2, but omit to
>>>> clean the closures that still point to 'hello'...
>>>> Thus, 'hello' is not finalized.
>>>> One possibility is to also clean the closures pointing to 'hello'.
>>>> But now the code is tricky... It's aim was to be simple damned :)
>>>>
>>>> Nicolas
>>>>
>>>>
>>>> here is a corrected version :
>>>>
>>>>
>>>> testFinalizationOfEquals
>>>>        "self debug: #testFinalizationOfEquals"
>>>>
>>>>        | finalizationProbe o1 o2
>>>>        forAnyTwoEqualObjects
>>>>        ofDifferentIdentity
>>>>        registeringAnActionAtFinalizationForEachObject
>>>>        thenForcingFinalizationOfObjects
>>>>        implyBothRegisteredActionsAreExecuted |
>>>>
>>>>        finalizationProbe := Set new.
>>>>        o1 := 'hello' copy.
>>>>        o2 := 'hello' copy.
>>>>        forAnyTwoEqualObjects := [o1 = o2].
>>>>        ofDifferentIdentity := [o1 ~~ o2].
>>>>        registeringAnActionAtFinalizationForEachObject := [
>>>>                o1 toFinalizeSend: #add: to: finalizationProbe  with: 'first object finalized'.
>>>>                o2 toFinalizeSend: #add: to: finalizationProbe  with: 'second object
>>>> finalized'].
>>>>        thenForcingFinalizationOfObjects := [
>>>>                forAnyTwoEqualObjects := ofDifferentIdentity :=
>>>> registeringAnActionAtFinalizationForEachObject := nil.
>>>>                o1 := o2 := nil. Smalltalk garbageCollect].
>>>>        implyBothRegisteredActionsAreExecuted := [finalizationProbe size = 2].
>>>>
>>>>        self
>>>>                assert: forAnyTwoEqualObjects;
>>>>                assert: ofDifferentIdentity;
>>>>                should: [
>>>>                        registeringAnActionAtFinalizationForEachObject value.
>>>>                        thenForcingFinalizationOfObjects value.
>>>>                        implyBothRegisteredActionsAreExecuted value].
>>>>
>>>
>>
>

>
>
> On Sat, Jan 16, 2010 at 12:33 PM, Nicolas Cellier
> <[hidden email]> wrote:
>>
>> I wrote this test pre-closure.
>> The problem with closures is that they are optimized to retain a
>> pointer to the values of outer temporary variable they use when
>> activated.
>
> Not exactly.
>
>>
>> Thus the BlockClosures forAnyTwoEqualObjects ofDifferentIdentity
>> registeringAnActionAtFinalizationForEachObject all refer to 'hello'
>> pointed by o1 and its copy pointed by o2.
>> In other words, o1 and o2 are not the single pointers to the String
>> 'hello' and its copy.
>
> They are.  See below.
>
>>
>> thenForcingFinalizationOfObjects only clean o1 and o2, but omit to
>> clean the closures that still point to 'hello'...
>> Thus, 'hello' is not finalized.
>> One possibility is to also clean the closures pointing to 'hello'.
>> But now the code is tricky... It's aim was to be simple damned :)
>
> There are single pointers to o1 and o2 from all the code in the test.  o1
> and o2 are stored in an indirect temp vector array created at the start of
> the method.  The compiler puts them there because there is an assignment to
> each after they are closed over:
>                o1 := o2 := nil. Smalltalk garbageCollect].
> All the blocks that reference o1 or o2 actually reference the indirect temp
> vector, so there is only one reference to o1 and o2 from all code in the
> method, though a single indirect temp vector.  So as soon as
>                o1 := o2 := nil.
> is executed the only references to o1 & o2 from code have been deleted.

>>
>> Nicolas
>>
>>
>> here is a corrected version :
>>
>>
>> testFinalizationOfEquals
>>        "self debug: #testFinalizationOfEquals"
>>
>>        | finalizationProbe o1 o2
>>        forAnyTwoEqualObjects
>>        ofDifferentIdentity
>>        registeringAnActionAtFinalizationForEachObject
>>        thenForcingFinalizationOfObjects
>>        implyBothRegisteredActionsAreExecuted |
>>
>>        finalizationProbe := Set new.
>>        o1 := 'hello' copy.
>>        o2 := 'hello' copy.
>>        forAnyTwoEqualObjects := [o1 = o2].
>>        ofDifferentIdentity := [o1 ~~ o2].
>>        registeringAnActionAtFinalizationForEachObject := [
>>                o1 toFinalizeSend: #add: to: finalizationProbe  with:
>> 'first object finalized'.
>>                o2 toFinalizeSend: #add: to: finalizationProbe  with:
>> 'second object
>> finalized'].
>>        thenForcingFinalizationOfObjects := [
>>                forAnyTwoEqualObjects := ofDifferentIdentity :=
>> registeringAnActionAtFinalizationForEachObject := nil.
>>                o1 := o2 := nil. Smalltalk garbageCollect].
>>        implyBothRegisteredActionsAreExecuted := [finalizationProbe size =
>> 2].
>>
>>        self
>>                assert: forAnyTwoEqualObjects;
>>                assert: ofDifferentIdentity;
>>                should: [
>>                        registeringAnActionAtFinalizationForEachObject
>> value.
>>                        thenForcingFinalizationOfObjects value.
>>                        implyBothRegisteredActionsAreExecuted value].
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>

> 2010/1/17 Eliot Miranda <[hidden email]>:
>>
>>
>> On Sat, Jan 16, 2010 at 12:33 PM, Nicolas Cellier
>> <[hidden email]> wrote:
>>>
>>> I wrote this test pre-closure.
>>> The problem with closures is that they are optimized to retain a
>>> pointer to the values of outer temporary variable they use when
>>> activated.
>>
>> Not exactly.
>>
>>>
>>> Thus the BlockClosures forAnyTwoEqualObjects ofDifferentIdentity
>>> registeringAnActionAtFinalizationForEachObject all refer to 'hello'
>>> pointed by o1 and its copy pointed by o2.
>>> In other words, o1 and o2 are not the single pointers to the String
>>> 'hello' and its copy.
>>
>> They are.  See below.
>>
>>>
>>> thenForcingFinalizationOfObjects only clean o1 and o2, but omit to
>>> clean the closures that still point to 'hello'...
>>> Thus, 'hello' is not finalized.
>>> One possibility is to also clean the closures pointing to 'hello'.
>>> But now the code is tricky... It's aim was to be simple damned :)
>>
>> There are single pointers to o1 and o2 from all the code in the test.  o1
>> and o2 are stored in an indirect temp vector array created at the start of
>> the method.  The compiler puts them there because there is an assignment to
>> each after they are closed over:
>>                o1 := o2 := nil. Smalltalk garbageCollect].
>> All the blocks that reference o1 or o2 actually reference the indirect temp
>> vector, so there is only one reference to o1 and o2 from all code in the
>> method, though a single indirect temp vector.  So as soon as
>>                o1 := o2 := nil.
>> is executed the only references to o1 & o2 from code have been deleted.
>
> Oh thanks, I get it, the tempVector is the only pointer to 'hello'
> because it is used both inside and outside blocks.
> Maybe the WeakKeyIdentityDictionary bug lead me to this false conclusion...
>
> Stef, Marcus, don't change this method, just correct issue 1839 (and
> remove the expected failure)
>
> Nicolas
>
>>>
>>> Nicolas
>>>
>>>
>>> here is a corrected version :
>>>
>>>
>>> testFinalizationOfEquals
>>>        "self debug: #testFinalizationOfEquals"
>>>
>>>        | finalizationProbe o1 o2
>>>        forAnyTwoEqualObjects
>>>        ofDifferentIdentity
>>>        registeringAnActionAtFinalizationForEachObject
>>>        thenForcingFinalizationOfObjects
>>>        implyBothRegisteredActionsAreExecuted |
>>>
>>>        finalizationProbe := Set new.
>>>        o1 := 'hello' copy.
>>>        o2 := 'hello' copy.
>>>        forAnyTwoEqualObjects := [o1 = o2].
>>>        ofDifferentIdentity := [o1 ~~ o2].
>>>        registeringAnActionAtFinalizationForEachObject := [
>>>                o1 toFinalizeSend: #add: to: finalizationProbe  with:
>>> 'first object finalized'.
>>>                o2 toFinalizeSend: #add: to: finalizationProbe  with:
>>> 'second object
>>> finalized'].
>>>        thenForcingFinalizationOfObjects := [
>>>                forAnyTwoEqualObjects := ofDifferentIdentity :=
>>> registeringAnActionAtFinalizationForEachObject := nil.
>>>                o1 := o2 := nil. Smalltalk garbageCollect].
>>>        implyBothRegisteredActionsAreExecuted := [finalizationProbe size =
>>> 2].
>>>
>>>        self
>>>                assert: forAnyTwoEqualObjects;
>>>                assert: ofDifferentIdentity;
>>>                should: [
>>>                        registeringAnActionAtFinalizationForEachObject
>>> value.
>>>                        thenForcingFinalizationOfObjects value.
>>>                        implyBothRegisteredActionsAreExecuted value].
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> [hidden email]
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> [hidden email]
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project