38776 - Potential Leak: OleUninitialize() isn't called for each OleInitialize()

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P3)

Description

[:deanis74]

nsWindow::StandardWindowCreate() calls OleInitialize() and sets gOLEInited = 
TRUE.  For each of these calls to OleInitialize, there should be a matching call 
to OleUnitialize.

To quote from MSDN 
(http://msdn.microsoft.com/library/psdk/com/ofn_ol2z_13vp.htm):

"The OleInitialize and OleUninitialize calls must be balanced — if there are 
multiple calls to the OleInitialize function, there must be the same number of 
calls to OleUninitialize: Only the OleUninitialize call corresponding to the 
OleInitialize call that actually initialized the library can close it."

Comment 1

[sairuh (rarely reading bugmail)]

jrgm, would you know how to narrow this one down? thx!

Comment 2

[:deanis74]

Whoops.  I'm dropping the status down to normal because I somehow missed that 
this is called only if gOLEInited is not TRUE.  But it still needs to be cleaned 
up somewhere, probably on app exit.

Comment 3

[John Morrison]

My best guess from lxr is that this is code that rods has worked with at least.
However, I'm not going to guess whether this is a problem, or what the solution 
would be. Wanna propose a patch, Dean?

Comment 4

[:deanis74]

There's a couple of things that can be done.  One is to call OleUnitialize() in 
nsWindow::Destroy() and reset the gOleInited flag, but that would then cause 
OleInitialize to be called with the next StandardWindowCreate().  This would 
make sure that each OleInit was paired with an OleUninit, but it would overdo it 
on the calls to OleInit.

The better way to do it would be when the app shuts down completely.  If 
gOleInited == TRUE, call OleUnitialize() and set gOleInited = FALSE.  But I'm 
not sure where in the code this would be done.

Comment 5

[rods (gone)]

gOLEInited is a global variable and OleInitialize is only called once.

marking invalid

Comment 6

[:deanis74]

OleUninitialize should still be called on app exit.

Comment 7

[rods (gone)]

OleUninitialize is not being called

Comment 8

[:deanis74]

I realize that.  My question is why isn't it being called?

Comment 9

[:deanis74]

Sorry Rod.  I missed that you re-opened the bug with your last comment.

Comment 10

[rods (gone)]

it now get properly inited and uninitied - fixed

Comment 11

[:deanis74]

Does OleRegisterMgr::~OleRegisterMgr() get called on shutdown?  Is there a way 
that it can reset the gOleInited variable?

Comment 12

[rods (gone)]

Because it is a global object it gets created very early on and then gets 
destroyed at the very end. I could have put a counter in nsWindow to count the 
number of constructs and destructs but decided that this approach was safer.

Comment 13

[:deanis74]

Well then there's still a little problem.  OleInitialize will get called twice 
and OleUninitialize only once.  The first time OleInit gets called is in 
OleRegisterMgr::OleRegisterMgr() and the second time is in the first 
nsWindow::StandardWindowCreate().  A suggestion is to get rid of the call in 
StandardWindowCreate() if OleRegisterMgr is always created very early on.

Comment 14

[John Morrison]

  > second time is in the first nsWindow::StandardWindowCreate()

That was removed in the same checkin that added the new class, no? There is only 
one call to OleInitialize possible.

Comment 15

[rods (gone)]

Yes, it was removed and now there is only one call to each

Comment 16

[:deanis74]

Stupid me.  Asked before I look.  Apologies.

Comment 17

[John Morrison]

Yeah, but it made me look :-]

Comment 18

[rods (gone)]

No, problem. I didn't realize we were never doing the uninit, thanks for the 
help.