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

Wed Mar 10 04:43:10 UTC 2010

On Wed, 10 Mar 2010, Igor Stasenko wrote:

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

For some edge cases.
I added a fix to the trunk. :)

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

I just tried to understand why your current implementation needs a 
WeakKeyDictionary, but I didn't.

Levente

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