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

Juan Vuletich juan at jvuletich.org
Tue Nov 29 22:41:24 UTC 2011


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.

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

> 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). 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.

> 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




More information about the Squeak-dev mailing list