[squeak-dev] [Bug][Fix] In ReferenceStream, code review request
Mariano Martinez Peck
marianopeck at gmail.com
Thu Dec 1 09:31:03 UTC 2011
On Tue, Nov 29, 2011 at 11:41 PM, Juan Vuletich <juan at jvuletich.org> wrote:
> Hi Mariano,
>
>
> Mariano Martinez Peck wrote:
>
>> Hi. I am not sure if I understand which is the bug and it is difficult to
>> do a clear diff. You mean that weak references should NOT be serialized ?
>>
>
> The test below documents the correct behavior. See implementors and
> senders of #nextPutWeak: in Cuis. Those methods have _very_ good comments.
> If you only dump a model, any views should not get dumped. If you dump a
> view, both view and model must be dumped. Basic View / Model separation:
> the View knows the Model, the Model doesn't know about the View. In the
> case of Morph, the owner of a morph dumped by itself should not be
> included; but the owner / submorphs relationship must be correct for all
> morphs that ultimately get dumped. The subtler bug in Cuis was in
> #references, and is also caught by this test.
>
Thanks, it is much clear now.
>
> While thinking about this in Fuel I asked in the mailing list <
>> http://forum.world.st/Should-**a-serializer-do-something-in-**
>> particular-with-weak-**references-td3827593.html<http://forum.world.st/Should-a-serializer-do-something-in-particular-with-weak-references-td3827593.html>>
>> and the conclusion was that we shouldn't do anything special in Fuel for
>> weak references.
>>
>
> Wrong conclusion. The correct answer was Levente's:
> http://forum.world.st/Should-**a-serializer-do-something-in-**
> particular-with-weak-**references-td3827593.html#**a3828288<http://forum.world.st/Should-a-serializer-do-something-in-particular-with-weak-references-td3827593.html#a3828288>..
>
>
Well, it is just "another" conclusion, but I don't think it is totally
wrong (more below)
>
> In this case, if I understand, you are saying the opposite: do something
>> different for weak references, basically to consider them "transient". Can
>> you please explain me why it is needed a specific behavior for weak
>> references?
>>
>
> Including objects that are only weakly referenced from within the dumped
> object graph (but strongly referenced from somewhere else) is useless: very
> shortly after materialization they will go away. You're wasting serialized
> file size storing garbage, and requesting that these objects can be
> materialized (their class or some appropriate replacement must be present,
> etc).
100% agree. That's why I was asking, because from your first answer it
looks like not doing that was WRONG or that there could be a real problem.
>From my point of view, it is valid, and it is a nice behavior, but it is an
optimization. So I see it as an "optimized solution" :)
We are probably going to do the same in Fuel.
> But a serialized model should be materializable in, for instance, an
> UI-less server, or some client with a very different UI framework, etc.
> This would enable (among many other useful things) sharing serialized
> objects between Cuis, Squeak and Pharo.
>
>
I understand what you mean but I think this concrete example is wrong.
Since the model doesn't know the view (hence the view is not serialized),
wouldn't it always be able to be materialized in a UI-less server?
Thanks!
> Thanks in advance!
>>
>
>
> Cheers,
> Juan Vuletich
>
> On Sun, Nov 27, 2011 at 6:28 PM, Juan Vuletich <juan at jvuletich.org<mailto:
>> juan at jvuletich.org>> wrote:
>>
>> Hi Folks,
>>
>> I found a bug in Cuis that made weak references to be improperly
>> dumped in SmartRefStreams due to a bug in ReferenceStream. When I
>> tried to reproduce the bug and fix in Squeak, I found out that in
>> Squeak weak references are always dumped (and not just the more
>> subtle bug in Cuis). The attach includes a test and fix. I'm
>> confident about the fix in the context of Cuis, but I only played
>> a few minutes with this in Squeak.
>>
>> Can anybody check this for Squeak and consider it for inclusion?
>>
>> Thanks,
>> Juan Vuletich
>>
>> ...
>>
>>
>>
>> !ReferenceStreamTest methodsFor: 'testing' stamp: 'jmv 11/27/2011
>> 14:56'!
>> testWeakDumps
>> "Test that if we serialize a model with weak references to
>> views, only the model is serialized and not the views.
>>
>> Note: The bug became apparent only when dumping a model to
>> a SmartRefStream, that calls #references, and the serialized stream
>> was later materialized in an image where the view classes
>> had been deleted. In such rare cases, materialization would fail
>> when trying to reference these
>> absent classes. If serializing to a ReferenceStream, the
>> bug didn't become apparent (views were never serialized). If
>> serializing to a SmartRefStream, but
>> view classes still existed, the bug didn't really become
>> apparent (because views were not actually deserialized), the only
>> effect was a larger file.
>>
>> ReferenceStreamTest new testWeakDumps
>> "
>> | oldInstance refStream |
>> oldInstance :=StringHolder new contents: 'This is a text'.
>> oldInstance addDependent: Morph new.
>> refStream := ReferenceStream on: (DummyStream on: nil).
>> refStream nextPut: oldInstance.
>> self deny: (refStream references keys anySatisfy: [
>> :dumpedObject | dumpedObject isKindOf: Morph ])! !
>>
>>
>>
>>
>>
>>
>>
>> --
>> Mariano
>> http://marianopeck.wordpress.**com <http://marianopeck.wordpress.com>
>>
>
>
>
--
Mariano
http://marianopeck.wordpress.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20111201/2a9edeef/attachment.htm
More information about the Squeak-dev
mailing list
|