Click here to Skip to main content
15,887,585 members

Comments by Phil Atkin (Top 5 by date)

Phil Atkin 2-Sep-11 7:22am View    
Deleted
You are right, and the article I referred to is wrong. I'm going to add an Alternative that demonstrates this and refines my Tip.
Phil Atkin 29-Aug-11 13:32pm View    
Deleted
You're very sure of yourself. I'm less sure. I tried a lot of things and failures were hard to reproduce. But the article I referred to _is_ sure, and it clearly states that the compiler-generated try-finally-dispose does _not_ prevent the object from being finalized before the Dispose. Look for the discussion around "Our mistake was to interpret the using clause not just as a way to have Dispose() called automatically but also as a way to prevent the “used” object from being garbage collected. Just because we think that the used object is valid till the end of the block does not mean that the garbage collector agrees with us!"
Thinking about this, it seems a reasonable design. The compilers analyse the code and determine that a particular object is collectable beyond a certain point. It is reasonable that this object is finalized at _any_ time after that point is passed, even if there is an explicit instruction later on to call Dispose. So the (implicit) call to Dispose does _not_ implicitly do a KeepAlive
Phil Atkin 29-Aug-11 6:25am View    
Deleted
I agree that it is curious that the KeepAlive call is necessary, and it would be better if this need had been avoided - especially since the consequences of omitting it are so difficult and complex. However, I believe it is _only_ required when the method calls into managed code. As I have mentioned in an earlier comment, the using construct reduces the risk of premature finalization but does NOT eliminate it. I do not know if an explicit call to Dispose would do it.
One of the changes I made following my discovery of this problem was to add a facility to my class library to detect failure to Dispose. When enabled for the class, an instantiation would retain details of the call stack, thread ID and time. On finalization, the object would log these details, enabling the faulty code to be discovered - whereas a Dispose would be silent.
Phil Atkin 29-Aug-11 6:14am View    
Deleted
I have omitted some details with the intention of keeping the size of the text down. However, some are instructive. In fact these objects deserve to be disposable (as you point out) and indeed they are. However, the use of using or an explicit invocation of Dispose reduces the probability of premature disposal, probably because it reduces the overall memory pressure, but does NOT eliminate it. There remains a chance that the garbage collector will run during the execution of WorkWith and will finalize it. I haven't tried your other approach of making the object a member but I would guess that it would effectively keep the object alive. However, it's legitimate to instantiate an object for temporary use, and to make it a member variable implies that it's not temporary - so I would argue that this approach would compromise the design of the code. However, my real objective in publishing this was to bring to light a most unexpected behaviour of the system; if people are aware of it - however they resolve it - that objective will be achieved.
Phil Atkin 22-Feb-11 8:41am View    
Deleted
The big reason the original is better is that the List<>.Contains operation has complexity O(n), whereas HashSet<>.Add will be O(Log(n)). That's going to make a big difference if we're dealing with 24 bits of colour data (so roughly 2^24/24 or 600,000 times faster).