Issue #1147

Issue #1147

This should be all you need. Just check the return values for null before instantiating the managed pointer types.

This code is for you. I'm not set up to replicate the defect at the moment. Download the patch and see if it works for you.

[Eric, 2014-07-31 15:37:08]

http://codereview.adblockplus.org/4642970720534528/diff/5629499534213120/inst...
File installer/src/installer-lib/process.cpp (right):

http://codereview.adblockplus.org/4642970720534528/diff/5629499534213120/inst...
installer/src/installer-lib/process.cpp:21: // OpenProcess does not return
INVALID_HANDLE VALUE, so a simple check is fine.
You can kill this comment in your own patch set.

[Eric, 2014-07-31 15:44:24]

Defect fix in comment

http://codereview.adblockplus.org/4642970720534528/diff/5629499534213120/inst...
File installer/src/installer-lib/process.cpp (right):

http://codereview.adblockplus.org/4642970720534528/diff/5629499534213120/inst...
installer/src/installer-lib/process.cpp:22: if (!h) return false;
OOPS! This should be 'return true'. If the process is no longer available, it
certainly can't possibly be immersive. This addresses the condition where the
processNames member has been queried at a previous time and the process has
terminated in the interim. 

This odd-looking statement certainly deserves a comment above it. It wasn't
obvious to me; I make an error the first time.

[Oleksandr, 2014-08-05 13:13:51]

This is part of the fix, but doesn't actually fix the MSIHANDLE leakage. Thanks
anyway. You can close this.

On 2014/07/31 15:44:24, Eric wrote:
> Defect fix in comment
> 
>
http://codereview.adblockplus.org/4642970720534528/diff/5629499534213120/inst...
> File installer/src/installer-lib/process.cpp (right):
> 
>
http://codereview.adblockplus.org/4642970720534528/diff/5629499534213120/inst...
> installer/src/installer-lib/process.cpp:22: if (!h) return false;
> OOPS! This should be 'return true'. If the process is no longer available, it
> certainly can't possibly be immersive. This addresses the condition where the
> processNames member has been queried at a previous time and the process has
> terminated in the interim. 
> 
> This odd-looking statement certainly deserves a comment above it. It wasn't
> obvious to me; I make an error the first time.

[Eric, 2014-08-05 13:30:39]

On 2014/08/05 13:13:51, Oleksandr wrote:
> This is part of the fix, but doesn't actually fix the MSIHANDLE leakage.
Thanks
> anyway. You can close this.

You're welcome.

I have a suspicion about what's wrong. It would be in 'class Record'. I mocked
up a move constructor with a proxy class for that (since I did not know at the
time that we had move available). In any case, 'Record' is used to create
argument lists for dialog boxes. It's possible that the proxy-move isn't
triggering the destructor correctly.

To test this, navigate through the dialogs using a path with a different number
of dialog boxes; since you can change your mind in them, this is possible. If
the number of leaked handles matches the number of dialogs, the leak is probably
in class Record. If it is, I'll do the fix for you.