Immutables and indexers and finalizers, oh my!
-
Here is a lovely one I hand coded earlier. (names changed and many innocuous lines removed to protect the innocent code around it.)
void ReduceCandidates(OblongCollection candidateOblongs)
{
for (int index = 0; index < candidateOblongs.Count; ++index)
{
Oblong fittedOblong = candidateOblongs[index].Reduce(); // <---------- Object Disposed exception here
// }
}This looks like a simple piece of code that nothing can go wrong with. Unfortunately it occasionally produces an object disposed exception on the line indicated. OblongCollection and Oblong are both immutable classes with finalizers and implementing the dispose pattern. What I think must be happening is a new Oblong is created in the OblongCollection indexer, the Oblong then goes out of scope and the Reduce method is called on it. In the Reduce method a method is called on the sole instance variable, but before the method is executed the finalizer kicks into action. This sees that the Oblong is no longer in scope, no instance variables are being used so disposes of the Oblong and calls the finalizer, trashing the object that the Reduce method is calling. Took me a while to figure out what was happening - I did not want to just blindly change the code. Fixed code below. (Yes, I know a foreach would be better here, if possible, but I want to keep this simple).
void ReduceCandidates(OblongCollection candidateOblongs)
{
for (int index = 0; index < candidateOblongs.Count; ++index)
{
using (Oblong currentCandidate = candidateOblongs[index])
{
Oblong fittedOblong = currentCandidate.Reduce();
// }
}
}Time to throw the Exception ElectrocuteCoder in every finalizer I guess ;-)
-
Here is a lovely one I hand coded earlier. (names changed and many innocuous lines removed to protect the innocent code around it.)
void ReduceCandidates(OblongCollection candidateOblongs)
{
for (int index = 0; index < candidateOblongs.Count; ++index)
{
Oblong fittedOblong = candidateOblongs[index].Reduce(); // <---------- Object Disposed exception here
// }
}This looks like a simple piece of code that nothing can go wrong with. Unfortunately it occasionally produces an object disposed exception on the line indicated. OblongCollection and Oblong are both immutable classes with finalizers and implementing the dispose pattern. What I think must be happening is a new Oblong is created in the OblongCollection indexer, the Oblong then goes out of scope and the Reduce method is called on it. In the Reduce method a method is called on the sole instance variable, but before the method is executed the finalizer kicks into action. This sees that the Oblong is no longer in scope, no instance variables are being used so disposes of the Oblong and calls the finalizer, trashing the object that the Reduce method is calling. Took me a while to figure out what was happening - I did not want to just blindly change the code. Fixed code below. (Yes, I know a foreach would be better here, if possible, but I want to keep this simple).
void ReduceCandidates(OblongCollection candidateOblongs)
{
for (int index = 0; index < candidateOblongs.Count; ++index)
{
using (Oblong currentCandidate = candidateOblongs[index])
{
Oblong fittedOblong = currentCandidate.Reduce();
// }
}
}Time to throw the Exception ElectrocuteCoder in every finalizer I guess ;-)
Is
OblongCollectionyour class? If so then the method should be a member of that. Otherwise, I'll just STFU.
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
-
Is
OblongCollectionyour class? If so then the method should be a member of that. Otherwise, I'll just STFU.
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
OblongCollectionandOblongare both my classes that wrap different unmanaged objects. If I put theReduce()method onOblongCollectionthen it would return anotherOblongCollectionand still need a loop to process the code that has been snipped out. Although saying that it should also be a method onOblongCollection, I have just not got around to doing it, as I have never needed it. -
OblongCollectionandOblongare both my classes that wrap different unmanaged objects. If I put theReduce()method onOblongCollectionthen it would return anotherOblongCollectionand still need a loop to process the code that has been snipped out. Although saying that it should also be a method onOblongCollection, I have just not got around to doing it, as I have never needed it.Member 2053006 wrote:
If I put the
Reduce()method onOblongCollectionthen it would return anotherOblongCollectionand still need a loop to process the code that has been snipped out.Sure?
public void ReduceCandidates()
{
for (int index = 0; index < this.Count; ++index)
{
using (Oblong currentCandidate = this[index])
{
Oblong fittedOblong = currentCandidate.Reduce();
//
}
}
}Your original never returned a new collection, so why should this?
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
-
Member 2053006 wrote:
If I put the
Reduce()method onOblongCollectionthen it would return anotherOblongCollectionand still need a loop to process the code that has been snipped out.Sure?
public void ReduceCandidates()
{
for (int index = 0; index < this.Count; ++index)
{
using (Oblong currentCandidate = this[index])
{
Oblong fittedOblong = currentCandidate.Reduce();
//
}
}
}Your original never returned a new collection, so why should this?
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
Ah, I think I see what you are saying there, move the entire method into
OblongCollection. A good idea, butOblongCollectionis in a shared library (mixed mode C++) and the snipped code is application specific, in C# and refers to some more instance members. I would rather keep application code out of the shared library unless it is generic enough to be useful in more then one application, which the snipped code is not. -
Ah, I think I see what you are saying there, move the entire method into
OblongCollection. A good idea, butOblongCollectionis in a shared library (mixed mode C++) and the snipped code is application specific, in C# and refers to some more instance members. I would rather keep application code out of the shared library unless it is generic enough to be useful in more then one application, which the snipped code is not.If that is the case, I'd wrap
OblongCollectionwithin an application class. It is, generally speaking, a good idea to always wrap external API's in a class as it hides the API specifics from the developer.
Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
-
Here is a lovely one I hand coded earlier. (names changed and many innocuous lines removed to protect the innocent code around it.)
void ReduceCandidates(OblongCollection candidateOblongs)
{
for (int index = 0; index < candidateOblongs.Count; ++index)
{
Oblong fittedOblong = candidateOblongs[index].Reduce(); // <---------- Object Disposed exception here
// }
}This looks like a simple piece of code that nothing can go wrong with. Unfortunately it occasionally produces an object disposed exception on the line indicated. OblongCollection and Oblong are both immutable classes with finalizers and implementing the dispose pattern. What I think must be happening is a new Oblong is created in the OblongCollection indexer, the Oblong then goes out of scope and the Reduce method is called on it. In the Reduce method a method is called on the sole instance variable, but before the method is executed the finalizer kicks into action. This sees that the Oblong is no longer in scope, no instance variables are being used so disposes of the Oblong and calls the finalizer, trashing the object that the Reduce method is calling. Took me a while to figure out what was happening - I did not want to just blindly change the code. Fixed code below. (Yes, I know a foreach would be better here, if possible, but I want to keep this simple).
void ReduceCandidates(OblongCollection candidateOblongs)
{
for (int index = 0; index < candidateOblongs.Count; ++index)
{
using (Oblong currentCandidate = candidateOblongs[index])
{
Oblong fittedOblong = currentCandidate.Reduce();
// }
}
}Time to throw the Exception ElectrocuteCoder in every finalizer I guess ;-)
-
It's a pretty big WTF that something can get disposed by the framework when you still have a reference to it.
There is no longer a reference to it when it is finalized. The indexer returns a new object which is not stored anywhere, so goes out of scope as soon as it is created. As soon as non of the instance fields of that object can not be accessed by any code the object is eligible for finalization. I could (and should) have prevented the finalizer from running by putting a GC.KeepAlive(this) at the end of the Reduce() method, yep, finialzers are nasty. The real code horror was not disposing of disposable objects. My bad. A good article I found on here that mentions GC.KeepAlive is Implementing IDisposable and the Dispose Pattern Properly[^] See point 2 in this article (OK, so this is for an old version of .net) (MSDN article) 3.9 Automatic memory management
-
There is no longer a reference to it when it is finalized. The indexer returns a new object which is not stored anywhere, so goes out of scope as soon as it is created. As soon as non of the instance fields of that object can not be accessed by any code the object is eligible for finalization. I could (and should) have prevented the finalizer from running by putting a GC.KeepAlive(this) at the end of the Reduce() method, yep, finialzers are nasty. The real code horror was not disposing of disposable objects. My bad. A good article I found on here that mentions GC.KeepAlive is Implementing IDisposable and the Dispose Pattern Properly[^] See point 2 in this article (OK, so this is for an old version of .net) (MSDN article) 3.9 Automatic memory management
There is still a reference – this within the executing Reduce method. While one could argue that you should have worked around it, the framework really shouldn't finalise something on which code is currently executing under any circumstances (well, unless you call Dispose before you try to do something else, I suppose).
-
Ah, I think I see what you are saying there, move the entire method into
OblongCollection. A good idea, butOblongCollectionis in a shared library (mixed mode C++) and the snipped code is application specific, in C# and refers to some more instance members. I would rather keep application code out of the shared library unless it is generic enough to be useful in more then one application, which the snipped code is not.Couldn't you use a delegate here? Pass a delegate for a void function taking an Oblong to ReduceCandidates(). Then you could implement the ReduceCandidates() in the library as a member of OblongCollection and the application specific code inside the delegate definition in the application.