On Fri, 24 May 2013, Frank Shearar wrote:

> On 24 May 2013 15:20, Levente Uzonyi <leves at elte.hu> wrote:
>> On Thu, 23 May 2013, Frank Shearar wrote:
>>
>>> On 14 May 2013 21:41, Levente Uzonyi <leves at elte.hu> wrote:
>>>>
>>>>
>>>> By chasing the pointers I found that some associations are stored in the
>>>> Environment's exports variable's namespace variable. That is the
>>>> IdentityDictionary which breaks during renaming, because it holds on the
>>>> same association.
>>>
>>>
>>> The test failure is also intermittent. Does that observation fit the
>>> hypothesis?
>>
>>
>> This is a bit more than a hypothesis, but yes, HashedCollections "heal"
>> themselves when they grow, shrink, get rehashed, or when the elements change
>> rapidly.
>>
>> I think the best solution to the problem is to change the way bindings are
>> handled. Currently bindings are used as associations in various
>> dictionaries, so changing their key has effect on other dictionaries. If the
>> bindings were the values of the dictionary and the bindings' keys were the
>> keys, then changing the binding would not break the dictionary itself.
>>
>> This alone doesn't fix the problems in Environments, where the same bindings
>> are stored in multiple dictionaries. Here's a snippet to find a binding
>> which is present in all dictionaries in an environment:
>>
>> | environment exportsNamespace importsNamespace contents bindings |
>> environment := Smalltalk globals.
>> exportsNamespace := (environment instVarNamed: #exports) instVarNamed:
>> #namespace.
>> importsNamespace := (environment instVarNamed: #imports) instVarNamed:
>> #namespace.
>> contents := environment instVarNamed: #contents.
>> bindings := environment instVarNamed: #bindings.
>> exportsNamespace associations detect: [ :each |
>>         (importsNamespace associationAt: each key ifAbsent: [ nil ]) == each
>>                 and: [ (contents associationAt: each key ifAbsent: [ nil ])
>> == each
>>                 and: [ (bindings associationAt: each key ifAbsent: [ nil ])
>> == each ] ] ].
>>
>> In my image it returns:
>> #MCReorganizationPreloader=>MCReorganizationPreloader
>
> Right. So in Environment >> #renameClass:from:to: we see this:
>
> <snip>
> oldref := self associationAt: oldName.
> contents removeKey: oldName.
> oldRef key: newName
> contents add: oldref
> <snip>
>
> and this might well be the cause of our grief? It should instead be
> something like
>
> cls := contents at: oldName.
> contents at:newName put: cls.
> contents removeKey: oldName.
>
> then?

It's not clear for me that in which case would you use this code, but it 
doesn't seem right in any of them.
If we keep the binding handling as it is now (the bindings are 
associations used by the dictionaries), then this is wrong, because it 
creates new bindings and doesn't update the old ones.
If we change the binding handling, so that the dictionaries 
contain name->binding associations, where binding is a name->class 
association, then this code is incomplete, because it doesn't change the 
binding's key.
In both cases the other 3 dictionaries have to be checked and updated if 
necessary: bindings, namespace in imports, and namespace in exports.
Btw, it's still not clear to me why is there a bindings and a contents 
dictionary, and how can a class be in all 4 dictionaries at the same 
time.


Levente

>
> frank
>
>> Levente
>>
>>
>>>
>>> frank
>>>
>>>> Levente
>>>>
>>>>
>>>>>
>>>>>
>>>>> Levente
>>>>>
>>>>>>
>>>>>> frank
>>>>>>
>>>>>>> Levente
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> On Sun, May 12, 2013 at 6:30 AM, Frank Shearar
>>>>>>>> <frank.shearar at gmail.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This test fails because, as far as I can tell, SystemChangeFileTest
>>>>>>>>>>>
>>>>>>>>> #testClassRenamed breaks the integrity of an IdentityDictionary.
>>>>>>>>>
>>>>>>>>> This IdentityDictionary (call it dict) reports having a key in
>>>>>>>>> #keysAndValuesDo: such that dict at: key raises a KeyNotFound.
>>>>>>>>>
>>>>>>>>> Inspecting dict verifies this weirdness, even though you can see the
>>>>>>>>> key in the inspector's right hand pane.
>>>>>>>>>
>>>>>>>>> Ideas?
>>>>>>>>>
>>>>>>>>> The diff in the manifests between the last version where this test
>>>>>>>>> didn't fail and where it did looks like this:
>>>>>>>>>
>>>>>>>>> $ diff /home/frank/Downloads/TrunkImage.313.manifest
>>>>>>>>> /home/frank/Downloads/TrunkImage.314.manifest
>>>>>>>>> 22c22
>>>>>>>>> < Monticello (bf.541)
>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Monticello (bf.542)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 26c26
>>>>>>>>> < MorphicTests (ar.18)
>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> MorphicTests (fbs.20)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 49c49
>>>>>>>>> < Tests (fbs.207)
>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> * Tests (fbs.212)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 62,63d61
>>>>>>>>>
>>>>>>>>> Not terribly enlightening...
>>>>>>>>>
>>>>>>>>> frank
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>