Issue 1147 - Fix installation when IE is opened

installer/src/installer-lib/process.cpp

 #include <stdexcept>
 #include <functional>
 #include <wctype.h>
 // <thread> is C++11, but implemented in VS2012
 #include <thread>
 
 #include "installer-lib.h"
 #include "process.h"
 #include "handle.h"
 #include "session.h"
 
 //-------------------------------------------------------
 //-------------------------------------------------------
 typedef int (__stdcall *IsImmersiveDynamicFunc)(HANDLE);
 bool process_by_any_exe_not_immersive::operator()( const PROCESSENTRY32W & process )
 {
   if (processNames.find(process.szExeFile) != processNames.end())
   {   
     // Make sure the process is still alive
-    Windows_Handle procHandle = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, process.th32ProcessID);
+    HANDLE tmpHandle = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, process.th32ProcessID);
+    if (tmpHandle == NULL)
+      return false;
+
+    Windows_Handle procHandle(tmpHandle);
     if ((procHandle == NULL) || (procHandle == INVALID_HANDLE_VALUE)) return false;

[Eric, 2014/08/08 14:14:58]

This line is redundant. The previous check on tmpHandle takes care of the NULL
case and OpenProcess doesn't return INVALID_HANDLE_VALUE (not according to the
documentation).

On the other hand, INVALID_HANDLE_VALUE is -1, so this line is also harmless.

If you don't want to remove the line for extra-safety, at least make a comment
that it's redundant.

[sergei, 2014/08/08 15:17:36]

- There is no reason to have tmpHandle.
- As Eric pointed, the non critical trouble here is only additional comparison
against INVALID_HANDLE_VALUE. just make the `if (...` line like this `if (NULL
== procHandle)`

[Eric, 2014/08/08 15:34:19]

No. See below.

 
     DWORD exitCode;
     if (!GetExitCodeProcess(procHandle, &exitCode)) return false;
 
     if (exitCode != STILL_ACTIVE) return false;
 
     // Check if this is a Windows Store app process (we don't care for IE in Modern UI)
-    Windows_Module_Handle user32Dll(LoadLibrary(L"user32.dll"));
+    HMODULE tmpModule = LoadLibrary(L"user32.dll");
+    if (tmpModule == NULL)
+      return true;
+    Windows_Module_Handle user32Dll(tmpModule);

[sergei, 2014/08/08 15:17:36]

Similar here. The problem was in the missed validation `if (NULL == user32Dll)`,
there is no need for tmpModule.

[Eric, 2014/08/08 15:34:19]

It would help to understand the class in question first.

The defect before was that there was no temporary variable. These handle classes
throw an exception when constructed with nullptr as the argument. The smart
pointer template is instantiated with a not-null policy.

[sergei, 2014/08/08 15:45:25]

But why? No one standard smart pointer or wrapper does it, such behaviour looks
superfluous restriction.

     if (!user32Dll) return true;
 
     IsImmersiveDynamicFunc IsImmersiveDynamicCall = (IsImmersiveDynamicFunc)GetProcAddress(user32Dll, "IsImmersiveProcess");
     if (!IsImmersiveDynamicCall) return true;
 
     BOOL retValue = !IsImmersiveDynamicCall(procHandle);
 
     return retValue;
   }
   return false;
@@ skipping 261 matching lines @@
       if ( ! is_running() )
       {
         return true ;
       }
     }
     // Assert is_running()
   }
   // No control path leaves the for-loop.
 } ;