At 06:41 PM 06/05/2006 -0400, Miguel de Icaza wrote: >Hello, > >> You probably mean this fix: >> >> http://lists.ximian.com/pipermail/mono-patches/2006-May/074236.html >> >> It doesn't fix the thread handles ref leaks I experienced. >> >> I was able to fix it by changing io-layer/threads.cs:GetCurrentThread >> to not ref the handle. I'm not sure about the side effects, still >> testing. > >Ah, I felt that would help. > >Taking the ref inside GetCurrentThread in my opinion is an incorrect >change. But I do not understand the ChangeLog entry nor why was that >patch supposed to fix the other bug (or if the other bug listed on the >SVN commit message is even the same fix).
As I understand it, the problem is that while certainly whenever a handle is created, it must represent a reference to the underlying object, the Windows GetCurrentThread() function doesn't actually create a handle. The comment block above the function in io-layer/threads.c reflects this (and it is, of course, documented in MSDN). If you create an actual handle but you don't increment the underlying object's reference count, then when the handle is closed, it'll decrement the reference count with no matching increment and the object will end up being released early, before the last handle is closed. What it boils down to is that it *is* correct behaviour for GetCurrentThread to not increase the reference count, but it is not correct behaviour for it to create a handle. Indeed, anyone following the MSDN documentation will not even bother to pass the handle GetCurrentThread returns into CloseHandle, so while in that situation, the object wouldn't end up being freed early as the reference count decrement in CloseHandle would never happen, a memory leak (albeit a small one) would occur (the size of whatever sits between a HANDLE and the underlying object). This leak is very likely what is causing the original poster's defect. The log message at the quoted URL also confuses me; it looks as though Gonzalo thinks io-layer's GetCurrentThread() is already matching Microsoft's pseudohandle behaviour. Certainly, though, the patch is correct if that code ever has to run on Windows (..and it'll also be correct if/when mono starts using pseudohandles too). :-) Jonathan Gilbert _______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
