[Pharo-project] Bug in #pointsTo: ?

Eliot Miranda eliot.miranda at gmail.com
Mon Jan 9 22:21:47 CET 2012


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

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gforge.inria.fr/pipermail/pharo-project/attachments/20120109/c52515a2/attachment.htm>


More information about the Pharo-project mailing list