[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
|