[squeak-dev] [Bug][Fix] In ReferenceStream, code review request

Levente Uzonyi leves at elte.hu
Tue Dec 6 03:29:52 UTC 2011


On Mon, 5 Dec 2011, David T. Lewis wrote:

> On Mon, Dec 05, 2011 at 05:02:57PM +0100, Levente Uzonyi wrote:
>> On Wed, 30 Nov 2011, David T. Lewis wrote:
>>>
>>> Given apparent agreement that this is the correct approach, and considering
>>> this as a bug fix (not new feature) for ReferenceStream, I have moved
>>> Juan's changes to trunk.
>>
>> After these changes we have five new errors when running all tests:
>>
>> BitmapStreamTests>>#testMatrixTransform2x3WithImageSegment
>> BitmapStreamTests>>#testShortIntegerArrayWithImageSegment
>> BitmapStreamTests>>#testShortPointArrayWithImageSegment
>> BitmapStreamTests>>#testShortRunArrayWithImageSegment
>> BitmapStreamTests>>#testWordArrayWithImageSegment
>>
>> The problem seems to be with DiskProxy's serialization, because the cause
>> of the error is that it's not found in the structures dictionary during
>> materialization.
>>
>
> Thanks for catching this.
>
> The problem is certainly in the serialization, because #testMatrixTransform2x3WithImageSegment
> should be writing a file called 'bitmapStreamTest.extSeg' of size 1549 bytes,
> but after the ReferenceStream changes the file is only 1380 bytes in size,
> indicating that something that should have been written to the file is
> now missing. I'm not sure of the cause though.

I found that the new implementation of ReferenceStream >> #references 
causes the problems. There are two different issues with it:
1) it doesn't return all non-weak references
2) it does return a copy

DiskProxy isn't serialized, because it's added to the references as 
DiskProxy -> #none, so it won't be returned by #references (1), therefore 
SmartRefStream >> #instVarInfo: won't add it to the structures.

Other classes are not serialized, because (e.g.) ImageSegment >> 
#storeDataOn: tries to modify aDataStream's references. If that method 
returns a copy (2), then the modification won't have any effect.

What can we do?

To solve (1) we have to make sure that all non-weak references are 
returned. It seems to me that weak references have an OrderedCollection as 
their value in the dictionary, but I don't know what's the reason for 
that. Anyway, using [ :value | value class ~~ OrderedCollection ] seems to 
be a possible solution. Another solution is to add a method that returns 
all references and use it from places where all of them is needed instead 
of #references.

To solve (2), the best thing to do is to add new methods to
ReferenceStream, allowing indirect manipulation of the contents of it's 
references variable and use these methods from ImageSegment. This would be 
more explicit than using a method that returns the references dictionary.


Levente

>
> Dave
>
>
>



More information about the Squeak-dev mailing list