On 10 March 2010 05:16, Igor Stasenko siguctua@gmail.com wrote:
On 10 March 2010 04:34, Levente Uzonyi leves@elte.hu wrote:
On Tue, 9 Mar 2010, Igor Stasenko wrote:
2010/3/9 Levente Uzonyi leves@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. 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.
oops, the line above should be:
registry add: array1; add: 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).
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.