[Pharo-project] WeakRegistry>>remove: - when you'll be in trouble
A current implementation of this method remove: oldObject ifAbsent: exceptionBlock "Remove oldObject as one of the receiver's elements." oldObject ifNil: [ ^nil ]. ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ]) ifNil: [ exceptionBlock value ] simply removes a previously registered object from registry and voila. Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc. Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it. Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have: - socket handle is closed - socket is wiped from weak registry - but weak subscription still listed somewhere in a list of subscriptions My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually. In other words: remove: oldObject ifAbsent: exceptionBlock "Remove oldObject as one of the receiver's elements." oldObject ifNil: [ ^nil ]. ^(self protected: [ | executor | executor := valueDictionary removeKey: oldObject ifAbsent: nil. executor discardFinalization. ]) ifNil: [ exceptionBlock value ] -- Best regards, Igor Stasenko AKA sig.
Moreover, i personally don't understand, why WeakRegistry have to be a subclass of Collection. What registry copy reject, select etc means to weak registry? My gut feeling that this is conceptually wrong. One should not use any kind of copying on weak registry. I think, that more intention revealing , one should send: registry discardFinalization: anObject instead of: registry remove: anObject On 10 October 2010 15:59, Igor Stasenko <siguctua@gmail.com> wrote:
A current implementation of this method
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ])         ifNil: [ exceptionBlock value ]
simply removes a previously registered object from registry and voila.
Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc.
Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it.
Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have:  - socket handle is closed  - socket is wiped from weak registry  - but weak subscription still listed somewhere in a list of subscriptions
My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually.
In other words:
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ | executor |       executor := valueDictionary removeKey: oldObject ifAbsent: nil.       executor discardFinalization.     ])     ifNil: [ exceptionBlock value ]
-- Best regards, Igor Stasenko AKA sig.
-- Best regards, Igor Stasenko AKA sig.
On Sun, 10 Oct 2010, Igor Stasenko wrote:
A current implementation of this method
remove: oldObject ifAbsent: exceptionBlock "Remove oldObject as one of the receiver's elements."
oldObject ifNil: [ ^nil ]. ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ]) ifNil: [ exceptionBlock value ]
simply removes a previously registered object from registry and voila.
Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc.
Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it.
Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have: - socket handle is closed - socket is wiped from weak registry - but weak subscription still listed somewhere in a list of subscriptions
My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually.
In other words:
remove: oldObject ifAbsent: exceptionBlock "Remove oldObject as one of the receiver's elements."
oldObject ifNil: [ ^nil ]. ^(self protected: [ | executor | executor := valueDictionary removeKey: oldObject ifAbsent: nil. executor discardFinalization. ]) ifNil: [ exceptionBlock value ]
It's only an issue with the new WeakRegistry implementation, previous implementations don't have such problem. I think changing the method as you suggested, implementing WeakFinalizerItem >> #discardFinalization as "executor := nil" and changing WeakFinalizerItem >> #finalizaValues to ignore the executor if it's nil will fix the problem. Am I right? I don't get how "multiple finalizers per object" is related to this problem at all. Levente
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On 10 October 2010 23:43, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 10 Oct 2010, Igor Stasenko wrote:
A current implementation of this method
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ])         ifNil: [ exceptionBlock value ]
simply removes a previously registered object from registry and voila.
Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc.
Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it.
Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have: - socket handle is closed - socket is wiped from weak registry - but weak subscription still listed somewhere in a list of subscriptions
My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually.
In other words:
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ | executor |       executor := valueDictionary removeKey: oldObject ifAbsent: nil.       executor discardFinalization.     ])     ifNil: [ exceptionBlock value ]
It's only an issue with the new WeakRegistry implementation, previous implementations don't have such problem. I think changing the method as you suggested, implementing WeakFinalizerItem >> #discardFinalization as "executor := nil" and changing WeakFinalizerItem >> #finalizaValues to ignore the executor if it's nil will fix the problem. Am I right?
I don't get how "multiple finalizers per object" is related to this problem at all.
No, you miss the point. When you removing object from weak registry, it is important , time to time to tell all of its executors, that they will no longer receive #finalize, because object is no longer a member of weak registry. If you simply set executor := nil, it does nothing, an executor itself did not notified that he won't be needed in any possible future. So, if your finalization action is to remove some object(s) from the list , you'll get your list dirty after object will die, because #finalize will be never sent to your executor. Here the simple test case: | coll obj someWrapper | coll := OrderedCollection new. obj := Object new. someWrapper := WeakArray with: obj. coll add: someWrapper. obj toFinalizeSend: #remove: to: coll with: someWrapper. obj finalizationRegistry remove: obj. obj := nil. Smalltalk garbageCollect. self assert: coll isEmpty the point is, that once you doing a #remove: , your finalization scheme is broken.
Levente
-- Best regards, Igor Stasenko AKA sig.
On Mon, 11 Oct 2010, Igor Stasenko wrote:
On 10 October 2010 23:43, Levente Uzonyi <leves@elte.hu> wrote: On Sun, 10 Oct 2010, Igor Stasenko wrote:
A current implementation of this method
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ])         ifNil: [ exceptionBlock value ]
simply removes a previously registered object from registry and voila.
Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc.
Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it.
Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have: - socket handle is closed - socket is wiped from weak registry - but weak subscription still listed somewhere in a list of subscriptions
My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually.
In other words:
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ | executor |       executor := valueDictionary removeKey: oldObject ifAbsent: nil.       executor discardFinalization.     ])     ifNil: [ exceptionBlock value ]
It's only an issue with the new WeakRegistry implementation, previous implementations don't have such problem. I think changing the method as you suggested, implementing WeakFinalizerItem >> #discardFinalization as "executor := nil" and changing WeakFinalizerItem >> #finalizaValues to ignore the executor if it's nil will fix the problem. Am I right?
I don't get how "multiple finalizers per object" is related to this problem at all.
No, you miss the point. When you removing object from weak registry, it is important , time to time to tell all of its executors, that they will no longer receive #finalize, because object is no longer a member of weak registry. If you simply set executor := nil, it does nothing, an executor itself did not notified that he won't be needed in any possible future. So, if your finalization action is to remove some object(s) from the list , you'll get your list dirty after object will die, because #finalize will be never sent to your executor. Here the simple test case: | coll obj someWrapper | coll := OrderedCollection new. obj := Object new. someWrapper := WeakArray with: obj. coll add: someWrapper. obj toFinalizeSend: #remove: to: coll with: someWrapper. obj finalizationRegistry remove: obj. obj := nil. Smalltalk garbageCollect. self assert: coll isEmpty the point is, that once you doing a #remove: , your finalization scheme is broken. I don't get you. When you remove an object from a WeakRegistry, you tell the system that you want the object removed and all related executors deleted. So the executors should never receive #finalize, they should be thrown away. Even if there's action to be done with the executors (which is pointless IMHO since those will be thrown away, but who knows), doing them is the responsibility of the sender of #remove:. Levente
Levente
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
2010/10/11 Levente Uzonyi <leves@elte.hu>:
On Mon, 11 Oct 2010, Igor Stasenko wrote:
On 10 October 2010 23:43, Levente Uzonyi <leves@elte.hu> wrote: On Sun, 10 Oct 2010, Igor Stasenko wrote:
A current implementation of this method
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ])         ifNil: [ exceptionBlock value ]
simply removes a previously registered object from registry and voila.
Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc.
Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it.
Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have: - socket handle is closed - socket is wiped from weak registry - but weak subscription still listed somewhere in a list of subscriptions
My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually.
In other words:
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ | executor |       executor := valueDictionary removeKey: oldObject ifAbsent: nil.       executor discardFinalization.     ])     ifNil: [ exceptionBlock value ]
It's only an issue with the new WeakRegistry implementation, previous implementations don't have such problem. I think changing the method as you suggested, implementing WeakFinalizerItem >> #discardFinalization as "executor := nil" and changing WeakFinalizerItem >> #finalizaValues to ignore the executor if it's nil will fix the problem. Am I right?
I don't get how "multiple finalizers per object" is related to this problem at all.
No, you miss the point. When you removing object from weak registry, it is important , time to time to tell all of its executors, that they will no longer receive #finalize, because object is no longer a member of weak registry.
If you simply set executor := nil, it does nothing, an executor itself did not notified that he won't be needed in any possible future. So, if your finalization action is to remove some object(s) from the list , you'll get your list dirty after object will die, because #finalize will be never sent to your executor.
Here the simple test case:
| coll obj someWrapper | coll := OrderedCollection new. obj := Object new. someWrapper := WeakArray with: obj. coll add: someWrapper.
obj toFinalizeSend: #remove: to: coll with: someWrapper. obj finalizationRegistry remove: obj. obj := nil. Smalltalk garbageCollect. self assert: coll isEmpty
the point is, that once you doing a #remove: , your finalization scheme is broken.
I don't get you. When you remove an object from a WeakRegistry, you tell the system that you want the object removed and all related executors deleted. So the executors should never receive #finalize, they should be thrown away. Even if there's action to be done with the executors (which is pointless IMHO since those will be thrown away, but who knows), doing them is the responsibility of the sender of #remove:.
Let us get back to multiple finalizers per object, when you have two separate places, which adding possibly different executors for a single object without knowing about each other (otherwise why would you want to add two finalizers instead of one). Now if one decides to do #remove: , how to ensure that second one won't be left with dirty/invalid state?
Levente
Levente
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- Best regards, Igor Stasenko AKA sig.
On Mon, 11 Oct 2010, Igor Stasenko wrote:
2010/10/11 Levente Uzonyi <leves@elte.hu>: On Mon, 11 Oct 2010, Igor Stasenko wrote:
On 10 October 2010 23:43, Levente Uzonyi <leves@elte.hu> wrote: On Sun, 10 Oct 2010, Igor Stasenko wrote:
A current implementation of this method
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ])         ifNil: [ exceptionBlock value ]
simply removes a previously registered object from registry and voila.
Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc.
Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it.
Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have: - socket handle is closed - socket is wiped from weak registry - but weak subscription still listed somewhere in a list of subscriptions
My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually.
In other words:
remove: oldObject ifAbsent: exceptionBlock     "Remove oldObject as one of the receiver's elements."
    oldObject ifNil: [ ^nil ].     ^(self protected: [ | executor |       executor := valueDictionary removeKey: oldObject ifAbsent: nil.       executor discardFinalization.     ])     ifNil: [ exceptionBlock value ]
It's only an issue with the new WeakRegistry implementation, previous implementations don't have such problem. I think changing the method as you suggested, implementing WeakFinalizerItem >> #discardFinalization as "executor := nil" and changing WeakFinalizerItem >> #finalizaValues to ignore the executor if it's nil will fix the problem. Am I right?
I don't get how "multiple finalizers per object" is related to this problem at all.
No, you miss the point. When you removing object from weak registry, it is important , time to time to tell all of its executors, that they will no longer receive #finalize, because object is no longer a member of weak registry.
If you simply set executor := nil, it does nothing, an executor itself did not notified that he won't be needed in any possible future. So, if your finalization action is to remove some object(s) from the list , you'll get your list dirty after object will die, because #finalize will be never sent to your executor.
Here the simple test case:
| coll obj someWrapper | coll := OrderedCollection new. obj := Object new. someWrapper := WeakArray with: obj. coll add: someWrapper.
obj toFinalizeSend: #remove: to: coll with: someWrapper. obj finalizationRegistry remove: obj. obj := nil. Smalltalk garbageCollect. self assert: coll isEmpty
the point is, that once you doing a #remove: , your finalization scheme is broken.
I don't get you. When you remove an object from a WeakRegistry, you tell the system that you want the object removed and all related executors deleted. So the executors should never receive #finalize, they should be thrown away. Even if there's action to be done with the executors (which is pointless IMHO since those will be thrown away, but who knows), doing them is the responsibility of the sender of #remove:.
Let us get back to multiple finalizers per object, when you have two separate places, which adding possibly different executors for a single object without knowing about each other (otherwise why would you want to add two finalizers instead of one). Now if one decides to do #remove: , how to ensure that second one won't be left with dirty/invalid state? It's not ensured, and it doesn't have to be. The sender of #remove: takes all responsibility. It can decide to add some executors back to the registry if that's necessary. If you want to keep your executors safe from some other code, which may #remove: your objects from a WeakRegistry, then you can create a private WeakRegistry and store your executors there, just like Sockets and FileStreams do. Levente
Levente
Levente
-- Best regards, Igor Stasenko AKA sig.
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Sig, Dumb question: how much does an executor need to know about this? It can't strongly bind the outgoing object or weaklings won't let go and finalization will never happen. Whether the guidance to it is in the form of "clean up" or "skip it," it needs to come from the weak registry, finalization framework, or whatever you want to call it. Dolphin handles finalization with support from the VM, allowing the outgoing object to do the finalization itself, often in the form of releasing external resources, so there is no concept of executor, let alone multiple executors. I have been arguing in favor of thread safety for these mechanisms, and I suspect this is another example of why it is needed. Why would one want more than one object ready to destroy an external handle when a single object disappears? The creation of the redundant executors is a recipe for for a double free and resulting crash. I think we should prevent such redundancy, and would be readily able to do so if the registries were thread safe giving #at:ifAbsentPut: the chops to do the job. Am I missing something? Bill ________________________________________ From: pharo-project-bounces@lists.gforge.inria.fr [pharo-project-bounces@lists.gforge.inria.fr] On Behalf Of Igor Stasenko [siguctua@gmail.com] Sent: Sunday, October 10, 2010 7:10 PM To: The general-purpose Squeak developers list Cc: Pharo Development Subject: Re: [Pharo-project] [squeak-dev] Re: WeakRegistry>>remove: - when you'll be in trouble On 10 October 2010 23:43, Levente Uzonyi <leves@elte.hu> wrote:
On Sun, 10 Oct 2010, Igor Stasenko wrote:
A current implementation of this method
remove: oldObject ifAbsent: exceptionBlock "Remove oldObject as one of the receiver's elements."
oldObject ifNil: [ ^nil ]. ^(self protected: [ valueDictionary removeKey: oldObject ifAbsent: nil ]) ifNil: [ exceptionBlock value ]
simply removes a previously registered object from registry and voila.
Now lets get back to our discussion about multiple finalizers per object and using them in weak subscriptions etc.
Suppose i am added a socket to weak registry, and suppose i am added a weak subscription to it.
Now, if i do 'socket close' , it tells weak registry to remove it from list. And what we'll have: - socket handle is closed - socket is wiped from weak registry - but weak subscription still listed somewhere in a list of subscriptions
My suggestion is, that upon #remove:, a weak registry should notify all executors that object of interest are no longer takes part in finalization scheme, so they should not count on receiving #finalize eventually.
In other words:
remove: oldObject ifAbsent: exceptionBlock "Remove oldObject as one of the receiver's elements."
oldObject ifNil: [ ^nil ]. ^(self protected: [ | executor | executor := valueDictionary removeKey: oldObject ifAbsent: nil. executor discardFinalization. ]) ifNil: [ exceptionBlock value ]
It's only an issue with the new WeakRegistry implementation, previous implementations don't have such problem. I think changing the method as you suggested, implementing WeakFinalizerItem >> #discardFinalization as "executor := nil" and changing WeakFinalizerItem >> #finalizaValues to ignore the executor if it's nil will fix the problem. Am I right?
I don't get how "multiple finalizers per object" is related to this problem at all.
No, you miss the point. When you removing object from weak registry, it is important , time to time to tell all of its executors, that they will no longer receive #finalize, because object is no longer a member of weak registry. If you simply set executor := nil, it does nothing, an executor itself did not notified that he won't be needed in any possible future. So, if your finalization action is to remove some object(s) from the list , you'll get your list dirty after object will die, because #finalize will be never sent to your executor. Here the simple test case: | coll obj someWrapper | coll := OrderedCollection new. obj := Object new. someWrapper := WeakArray with: obj. coll add: someWrapper. obj toFinalizeSend: #remove: to: coll with: someWrapper. obj finalizationRegistry remove: obj. obj := nil. Smalltalk garbageCollect. self assert: coll isEmpty the point is, that once you doing a #remove: , your finalization scheme is broken.
Levente
-- Best regards, Igor Stasenko AKA sig. _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Sun, Oct 10, 2010 at 08:16:57PM -0400, Schwab,Wilhelm K wrote:
<snip>
The creation of the redundant executors is a recipe for for a double free and resulting crash. <snip>
Am I missing something?
There is no such thing as a "double free and resulting crash". Dave
Dave, Since when? Even if operating systems started universally covering for such mistakes, I still would not recommend testing them on it. Bill ________________________________________ From: pharo-project-bounces@lists.gforge.inria.fr [pharo-project-bounces@lists.gforge.inria.fr] On Behalf Of David T. Lewis [lewis@mail.msen.com] Sent: Sunday, October 10, 2010 9:54 PM To: Pharo-project@lists.gforge.inria.fr Subject: Re: [Pharo-project] [squeak-dev] Re: WeakRegistry>>remove: - when you'll be in trouble On Sun, Oct 10, 2010 at 08:16:57PM -0400, Schwab,Wilhelm K wrote:
<snip>
The creation of the redundant executors is a recipe for for a double free and resulting crash. <snip>
Am I missing something?
There is no such thing as a "double free and resulting crash". Dave _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
participants (4)
-
David T. Lewis -
Igor Stasenko -
Levente Uzonyi -
Schwab,Wilhelm K