Re: mshtml: suppress module unloading while gecko is loaded
Dan Kegel wrote: Thanks to Mike Hearn for pointing out that mshtml reference counting was wrong, and Jacek for pointing out how to fix it for now. Once gecko is loaded, don't let mshtml unload. This fixes a crash in starting Sketchup (bug 16164), and probably fixes a number of other apps that went pear-shaped after unloading mshtml... Bug 14065 is another instance of the same thing. I did a quick survey of DllCanUnloadNow() on the Wine 1.1.8 code base a couple of weeks ago ... 55 DllCanUnloadNow routines (in some 272 dll subdirectories) 11 FIXME stubs return S_FALSE 7 FIXME stubs return S_OK 3 without FIXME return S_FALSE 2 without FIXME return S_OK 32 appear to be real implementations However, the first in the list (alphabetically) is in astream/main.c. It's reference counter (ddl_ref) is not incremented or decremented anywhere. I count four pairs of inc/dec omissions. In mshtml, I count 7 dll reference count inc/dec pairs, including the server lock pair, in main.c, htmldoc.c and protocol.c I count 28 pairs of InterlockedDecrement / InterlockedIncrement calls around the creation / destruction of active objects throughout the dll. That would seem to imply 22 missing reference count inc/dec pairs. If I finish my script, it will report how many pairs are missing for which dlls and in which source files, which might help make a decision whether the take up Mike Hearn's suggestion of never unloading anything. Even that should involve work taking out the reference counts that are already there. I do not see how the classic WinAPI regression tests can catch this kind of bug other than being a tautologous test of the creation/destruction all the known active objects. If Mike Hearn's suggestion is not taken up, the script might also be useful for regression test purposes. Cheers,
Re: mshtml: suppress module unloading while gecko is loaded
On Mon, Dec 1, 2008 at 12:08 AM, Paul Bryan Roberts [EMAIL PROTECTED] wrote: In mshtml, I count 7 dll reference count inc/dec pairs, including the server lock pair, in main.c, htmldoc.c and protocol.c I count 28 pairs of InterlockedDecrement / InterlockedIncrement calls around the creation / destruction of active objects throughout the dll. That would seem to imply 22 missing reference count inc/dec pairs. I suspect Alexandre didn't commit my lame patch because he wants us to have a go at getting CanDLLUnloadNow right. Thanks for the analysis. If I finish my script, it will report how many pairs are missing for which dlls and in which source files That sounds quite handy. which might help make a decision whether the take up Mike Hearn's suggestion of never unloading anything. Even that should involve work taking out the reference counts that are already there. I have a feeling we're not quite in the memory is free era yet... I do not see how the classic WinAPI regression tests can catch this kind of bug other than being a tautologous test of the creation/destruction all the known active objects. If Mike Hearn's suggestion is not taken up, the script might also be useful for regression test purposes. Your script is doing static analysis ... maybe it should be integrated into http://people.redhat.com/mstefani/wine/smatch/ Making it a smatch rule might be easier than writing it from scratch. - Dan
Re: mshtml: suppress module unloading while gecko is loaded (take 2)
Dan Kegel wrote: Thanks to Mike Hearn for pointing out that mshtml reference counting was wrong, and Jacek for pointing out how to fix it for now. Once gecko is loaded, don't let mshtml unload. This fixes a crash in starting Sketchup (bug 16164), and probably fixes a number of other apps that went pear-shaped after unloading mshtml... C++ style comment? Was this a Patchwatcher test ;) -- Cheers, Paul.
