[Vm-dev] [ENH] A better finalization support (VM & language side)

[Igor Stasenko]

2010/3/10 Levente Uzonyi <leves at elte.hu>:
>
> On Wed, 10 Mar 2010, Igor Stasenko wrote:
>
>>
>> On 10 March 2010 04:34, Levente Uzonyi <leves at elte.hu> wrote:
>>>
>>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>>
>>>>
>>>> 2010/3/9 Levente Uzonyi <leves at elte.hu>:
>>>>>
>>>>> On Tue, 9 Mar 2010, Igor Stasenko wrote:
>>>>>
>>>>>> Well, if someone cares, then he actually can make own registry class,
>>>>>> which allows copying. But why we should care, by leaving a potential
>>>>>> security hole open?
>>>>>> I don't think that this is normal to rely on good manners of users and
>>>>>> expect them to not attempt to do something wrong. In contrast, a
>>>>>> protocols and interfaces should discourage user from abuse. At least,
>>>>>> an author could state where it is safe to use and where is not.
>>>>>> A copy protocol for weak registry is far from being safe.
>>>>>> In that situation is better to generate an error, indicating that such
>>>>>> use is not foreseen by author, rather than trying to implement
>>>>>> something which 'possibly could work' :)
>>>>>
>>>>> I still don't see how is it unsafe.
>>>>>
>>>>
>>>> A simple copy is fine. A copy, which then registered in weak
>>>> dependents creating a problem.
>>>>
>>>> I even think that weak registry should not behave as collection at all,
>>>> and having only #add: method, with no ability to remove items.
>>>>
>>>> The only use of #remove: i see is in Socket and StandardFileStream,
>>>> which implement #unregister: in own class side.
>>>>
>>>> Now , if you look at my implementation, you could see that there is a
>>>> way to completely avoid the need in removing items from a
>>>> valueDictionary which is pairs of  <weakref> -> <executor>.
>>>>
>>>> A solution:
>>>> - add a 'finalizer' ivar to Socket/StandardFileStream
>>>> - by registering a socket in registry, retrieve an instance of
>>>> WeakFinalizerItem as a result of registration and store it into
>>>> 'finalizer' ivar.
>>>>
>>>> - on #destroy, simply nil-out all of the finalizer's ivars, so there
>>>> is no chances that once socket become garbage, it will trigger an
>>>> executor's #finalize method, which were registered previously in
>>>> registry.
>>>>
>>>> - forget about removing the finalizer manually from registry, because
>>>> an instance of WeakFinalizerItem which is held by 'valueDictionary' in
>>>> registry will eventually be reclaimed, once dictionary will discover
>>>> that corresponding key is nil.
>>>>
>>>> What you think?
>>>
>>> I took a deeper look and found that WeakFinalizationRegistry doesn't support
>>> multiple finalizers per object. It does what WeakRegistry did before: throw
>>> away the previous finalizer and replace it with a new one.
>>>
>>> I like the current features of WeakRegistry (removing, adding, multiple
>>> finalizers per object) and I think it's easy to modify it to use your vm
>>> support.
>>>
>>
>> Sure, support for multiple finalizers per object could be added. But
>> as to me, this looks like an over-engineering.
>>
>> Btw, the current implementation of WeakRegistry is buggy, because of
>> use non-identity based dictionary.
>
> Buggy is a bit strong, it just doesn't support objects which don't implement #= as #== and #hash as #scaledIdentityHash. But I already responded to this here: http://lists.squeakfoundation.org/pipermail/vm-dev/2010-March/003960.html
>

Okay. It is not buggy. It is wrong.  :)

>> Try explore contents of it, after evaluating:
>>
>> 10 timesRepeat: [ registry add: (Array with:10) ].
>> it adds only a single key/value pair into valueDictionary,
>> while i'd expect to have 10 entries, because i adding 10 distinct array objects.
>> The flaw in logic is easy to illustrate:
>>
>> array1 := Array with: 10.
>> array2 := Array with: 10.
>>
>> registry add: array1; array2.
>> array2 at: 1 put: 12.
>> registry add: array2.
>>
>> array1 := nil "remove strong reference to array1, while keep it for array2"
>>
>> The point is, that if two disctinct objects answering true on #= ,
>> when we adding them to registry
>> it doesn't means that they will keep to be 'equal' after we added
>> them, because these objects can mutate.
>> So, if one of them eventually die, other(s) may still be in use, which
>> will lead to receiving a death notice to wrong recipient.
>>
>>> I don't really like your suggestion for files and sockets, but it's doable.
>>>
>>> I was thinking about a simpler registry which would use an OrderedCollection
>>> instead of a WeakKeyDictionary. It'd need a new instance variable in
>>> WeakFinalizerItem (though it can be another class/subclass too) which would
>>> store the index of the finalizer in this OrderedCollection. It wouldn't have
>>> #do:, #keys or #remove:ifAbsent: (though all of them could be implemented)
>>> and #add: wouldn't replace existing finalizers, but just add them to the
>>> registry.
>>> This would have a bit better performance, simpler implementation and less
>>> features. But if you don't need #remove:, i'm sure it'd fit your needs.
>>> (It's a bit tricky though. Your socket/file ideas wouldn't work out of the
>>> box, because all of the WeakFinalizerItems would have to be removed from the
>>> OrderedCollecion somehow to avoid leaking memory. One solution would be to
>>> add it to the list on "remove" (when replacing the ivars with nil), another
>>> would be to remove it from the OrderedCollection by index, but the items
>>> don't know their collection so it would need another ivar).
>>>
>>> All in all, I'd keep WeakRegistry with the current features and optionally
>>> add a new simpler and faster registry if needed.
>>>
>>
>> Yes, i'm also thought about different ways how to organize things in
>> less memory & CPU hungry manner.
>> The main role of weak registry is to receive a notification, when
>> particular object become garbage.
>> Obviously, for this, we need to store a reference to notification
>> handler (also known as executor) somewhere to
>> be able to handle it. That's why current implementation using WeakKeyDictionary.
>> Once such notification is handled, we usually no longer need to keep
>> this object as well (thus we have to remove/unlink it from registry).
>
> I didn't see any code in your changesets that changed the way how weak registries are notified when their objects are gone. Did I miss something?
> If not, then #finalizeValues is still sent by the finalization process of WeakArray when it's semaphore is signaled, so one just has to register it's registry there.
>

Well, looks like you paying too much attention to implementation
details and don't see the whole idea behind this.
Its not really matter in what way you receiving such notification ,
what matters is that sometimes you need to be able to receive
a notification when some object become garbage. This is the reason why
we having a WeakRegistry.

>
> Levente
>
>>
>> My implementation is not perfect, it was mainly a quick and dirty hack
>> (you pointed on not using semaphores already).
>> But my intent was mainly to show how to use new VM feature, i added.
>> So, once it will be approved (i hope) and adopted, we could implement
>> things properly.
>>
>>>
>>> Levente
>>>
>>
>> --
>> Best regards,
>> Igor Stasenko AKA sig.
>
>



-- 
Best regards,
Igor Stasenko AKA sig.