[Pharo-project] Bug in #pointsTo: ?

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Mon Jan 9 17:00:50 EST 2012


2012/1/9 Eliot Miranda <eliot.miranda at gmail.com>:
>
>
> On Mon, Jan 9, 2012 at 1:16 PM, Mariano Martinez Peck
> <marianopeck at gmail.com> wrote:
>>
>> Ok guys....what about the following:
>>
>> stronglyPointsTo: anObject
>>
>>     "Answers true if the garbage collector would fail to collect anObject
>> because I hold a reference to it, or false otherwise"
>>
>>     (self instVarsInclude: anObject)
>>         ifTrue: [
>>             self class isWeak ifFalse: [ ^true ].
>>             1 to: self class instSize do: [ :i |
>>                 (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
>>             ^false ]
>>         ifFalse: [ ^self class == anObject and: [ self class isCompact not
>> ] ]
>
>
> Named inst vars in weak arrays are strong references.  So the above looks
> wrong to me. Its more like this:
>
>     1 to: self class instSize do:
>         [:i| (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
>     self class isWeak ifFalse:
>         [1 to: self basicSize do:
>             [:i| (se;f basicAt: i) == anObject ifTrue: [ ^true ] ].
>     ^false
>


But instVarIncludes: does walk over all the indexed slots, not just
the inst vars (yeah, the name does not tell)

  1 to: self class instSize do:
        [:i| (self instVarAt: i) == anObject ifTrue: [ ^true ] ].
  1 to: self basicSize do:
       [:i| (self basicAt: i) == anObject ifTrue: [ ^true ] ].
    ^false

So I think this part of the method works...

Only the class test looks strange, (WeakArray new) does point strongly
to its class like Henrik mentioned with his (WeakArray with:
WeakArray) joke - or I didn't understand ;)

We could also add a self basicSize = 0 ifTrue: [^true] before any
iteration, but better use new primitive indeed...

Nicolas

> This should be a primitive.  I believe Levente's primitive is even more
> useful and equivalent to
>
>     1 to: self class instSize do:
>         [:i| (self instVarAt: i) == anObject ifTrue: [ ^i ] ].
>     self class isWeak ifFalse:
>         [1 to: self basicSize do:
>             [:i| (se;f basicAt: i) == anObject ifTrue: [ ^i + self class
> instSize ] ].
>     ^0
>


>>
>>
>> pointsTo: anObject
>>     "Answers true if I hold a reference to anObject, or false otherwise"
>>
>>     ^ (self instVarsInclude: anObject)
>>         or: [ ^self class == anObject and: [ self class isCompact not ] ]
>>
>>
>> And then, from the the tracing pointer stuff (such as #pointersToExcept:)
>> we use #stronglyPointsTo
>>
>> do you agree?
>>
>>
>>
>> On Mon, Jan 9, 2012 at 9:55 PM, Eliot Miranda <eliot.miranda at gmail.com>
>> wrote:
>>>
>>>
>>>
>>> On Mon, Jan 9, 2012 at 12:47 PM, Eliot Miranda <eliot.miranda at gmail.com>
>>> wrote:
>>>>
>>>>
>>>>
>>>> On Mon, Jan 9, 2012 at 12:38 PM, Henrik Johansen
>>>> <henrik.s.johansen at veloxit.no> wrote:
>>>>>
>>>>>
>>>>> On Jan 9, 2012, at 8:46 06PM, Levente Uzonyi wrote:
>>>>>
>>>>> > On Mon, 9 Jan 2012, Mariano Martinez Peck wrote:
>>>>> >
>>>>> >>>
>>>>> >>>>>
>>>>> >>>>> So..I am puzzle again. I said "In which scenario can     (self
>>>>> >>>> instVarsInclude: anObject)  answer true, but the loop false? "
>>>>> >>>> you answered: "The scenario happens when the receiver has weak
>>>>> >>>> slots and
>>>>> >>>> the argument is referenced from one of those weak slots, but not
>>>>> >>>> from the
>>>>> >>>> other slots."
>>>>> >>>> Imagine the receiver has a weak slot XXX that points to anObject.
>>>>> >>>> So (self
>>>>> >>>> instVarsInclude: anObject) answers true.   How can the loop answer
>>>>> >>>> false
>>>>> >>>> without a GC?
>>>>> >>>> why would XXX stop pointing to anObject if there is not GC in the
>>>>> >>>> middle ?
>>>>> >>>>
>>>>> >>>
>>>>> >>> The loop doesn't iterate over the indexable slots, only named ones.
>>>>> >>>
>>>>> >>>
>>>>> >> grrr you are right!
>>>>> >>
>>>>> >> Ok, so I finally got it. So previously pointersTo: would answer true
>>>>> >> even
>>>>> >> if the slots were weak. Now it would answer false, and that's why
>>>>> >> you have
>>>>> >> changed the method comment.
>>>>> >> Now I am thinking if this should be the default behavior of
>>>>> >> #pointsTo:. If
>>>>> >> I just read the selector #pointsTo:  I would guess weak references
>>>>> >> are also
>>>>> >> taken into account.  So that's why I am not completely sure. Aren't
>>>>> >> there
>>>>> >> any other impact of the system because of this change?
>>>>> >> what about having #stronglyPointsTo: with this new version and have
>>>>> >> another
>>>>> >> one #pointsTo: which considers weak also?
>>>>> >> does it make sense?  or it is better to directly avoid weak by
>>>>> >> defualt in
>>>>> >> #pointsTo?
>>>>> >
>>>>> > I wasn't sure about this, that's why it's not in Squeak yet. Ignoring
>>>>> > weak references is okay as long as these methods are only used for pointer
>>>>> > tracing.
>>>>> >
>>>>> >
>>>>> > Levente
>>>>>
>>>>> Even with a good comment, the naming starts to make little sense, imho…
>>>>> Does an object having weak slots mean it no longer pointsTo: the
>>>>> objects in those slots?
>>>>>
>>>>> Sadly, I have no better succinct suggestions. :/
>>>>>
>>>>> Also, what happens when an object holds its (non-compact) class in a
>>>>> weak slot?
>>>>>
>>>>> In other words, is:
>>>>>
>>>>> wa := WeakArray new: 1.
>>>>> wa at: 1 put: WeakArray.
>>>>> self assert: (wa pointsTo: WeakArray).
>>>>>
>>>>> a valid test?
>>>>
>>>>
>>>> It is vacuous, since WeakArray is referred to indirectly from the roots
>>>> via Smalltalk and so will not go away.  The following is not a valid test
>>>> because there could be a GC in between the at:put: and the assert:.
>>>>
>>>> | o oh |
>>>> wa := WeakArray new: 1.
>>>> o := Object new.
>>>> oh := o hash.
>>>> wa at: 1 put: o.
>>>> o := nil.
>>>> self assert: (wa at: 1) hash = oh
>>>>
>>>> but 99 times out of a hundred it'll pass :)
>>>
>>>
>>> and it can better be written
>>>
>>> | oh |
>>> wa := WeakArray new: 1.
>>> oh := (wa at: 1 put: Object new) hash.
>>> self assert: (wa at: 1) hash = oh
>>>
>>>>
>>>> Weak arrays are tricky beasts.  To me, it doesn't matter that when used
>>>> on WeakArray pointsTo: or instVarsIncludes: or whatever produce results that
>>>> may be invalid at some subsequent time.  The same is true for e.g. a normal
>>>> Array that could be updated in some other thread.  It does mater that at the
>>>> point when an inst var is queried these methods/primitives answer the right
>>>> answer.  Writing valid tests that verify these answers is another matter
>>>> entirely.  Don't let the perfect be the enemy of the good.
>>>>
>>>>>
>>>>>
>>>>> Cheers,
>>>>> Henry
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> best,
>>>> Eliot
>>>>
>>>
>>>
>>>
>>> --
>>> best,
>>> Eliot
>>>
>>
>>
>>
>> --
>> Mariano
>> http://marianopeck.wordpress.com
>>
>
>
>
> --
> best,
> Eliot
>




More information about the Pharo-dev mailing list