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