Bug 453185: nsIProcess run fails with NS_ERROR_FILE_EXECUTION_FAILED when running an exe with a manifest that specifies requestedExecutionLevel level=requireAdministrator on Vista. r=bsmedberg
authorDave Townsend <dtownsend@oxymoronical.com>
Wed, 18 Mar 2009 11:00:20 +0000
changeset 26297 1f3d5e332a4429052cc629b6cb704bdd57f00c15
parent 26296 5092d98e31ad16bb45a8920539840c262cb50566
child 26298 7dd5b9ceebc513f37a50aa8615cb7ddedd36578b
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg
bugs453185
milestone1.9.2a1pre
Bug 453185: nsIProcess run fails with NS_ERROR_FILE_EXECUTION_FAILED when running an exe with a manifest that specifies requestedExecutionLevel level=requireAdministrator on Vista. r=bsmedberg
xpcom/tests/unit/test_nsIProcess.js
xpcom/threads/nsProcess.h
xpcom/threads/nsProcessCommon.cpp
--- a/xpcom/tests/unit/test_nsIProcess.js
+++ b/xpcom/tests/unit/test_nsIProcess.js
@@ -129,13 +129,48 @@ function test_arguments()
   var args= ["mozilla"];
   
   process.run(true, args, args.length);
   
   // exit codes actually seem to be unsigned bytes...
   do_check_neq(process.exitValue, 255);
 }
 
+var gProcess;
+
+// test if we can get an exit value from an application that is
+// run non-blocking
+function test_nonblocking()
+{
+  var testapp = filePrefix + "TestQuickReturn" + fileSuffix;
+  
+  var file = Components.classes["@mozilla.org/file/local;1"]
+                       .createInstance(Components.interfaces.nsILocalFile);
+  file.initWithPath(testapp);
+  
+  gProcess = Components.classes["@mozilla.org/process/util;1"]
+                       .createInstance(Components.interfaces.nsIProcess);
+  gProcess.init(file);
+
+  gProcess.run(false, [], 0);
+
+  do_test_pending();
+  do_timeout(100, "check_nonblocking()");
+}
+
+function check_nonblocking()
+{
+  if (gProcess.isRunning) {
+    do_timeout(100, "check_nonblocking()");
+    return;
+  }
+
+  do_check_eq(gProcess.exitValue, 42);
+  do_test_finished();
+}
+
 function run_test() {
   test_kill();
   test_quick();
   test_arguments();
+  if (isWindows)
+    test_nonblocking();
 }
--- a/xpcom/threads/nsProcess.h
+++ b/xpcom/threads/nsProcess.h
@@ -45,16 +45,17 @@
 #endif
 
 #include "nsIProcess.h"
 #include "nsIFile.h"
 #include "nsString.h"
 #include "prproces.h"
 #if defined(PROCESSMODEL_WINAPI) 
 #include <windows.h>
+#include <shellapi.h>
 #endif
 
 #define NS_PROCESS_CID \
 {0x7b4eeb20, 0xd781, 0x11d4, \
    {0x8A, 0x83, 0x00, 0x10, 0xa4, 0xe0, 0xc9, 0xca}}
 
 class nsProcess : public nsIProcess
 {
@@ -68,15 +69,16 @@ public:
 private:
   ~nsProcess();
 
   nsCOMPtr<nsIFile> mExecutable;
   PRInt32 mExitValue;
   nsCString mTargetPath;
 
 #if defined(PROCESSMODEL_WINAPI) 
-  PROCESS_INFORMATION procInfo;
+  typedef DWORD (WINAPI*GetProcessIdPtr)(HANDLE process);
+  HANDLE mProcess;
 #else
   PRProcess *mProcess;
 #endif
 };
 
 #endif
--- a/xpcom/threads/nsProcessCommon.cpp
+++ b/xpcom/threads/nsProcessCommon.cpp
@@ -50,63 +50,56 @@
 #include "nsProcess.h"
 #include "prtypes.h"
 #include "prio.h"
 #include "prenv.h"
 #include "nsCRT.h"
 
 #include <stdlib.h>
 
-#if defined(XP_WIN)
+#if defined(PROCESSMODEL_WINAPI)
 #include "prmem.h"
 #include "nsString.h"
 #include "nsLiteralString.h"
 #include "nsReadableUtils.h"
-#include <windows.h>
 #else
 #include <sys/types.h>
 #include <signal.h>
 #endif
 
 //-------------------------------------------------------------------//
 // nsIProcess implementation
 //-------------------------------------------------------------------//
 NS_IMPL_ISUPPORTS1(nsProcess, nsIProcess)
 
 //Constructor
 nsProcess::nsProcess()
-    : mExitValue(-1)
+    : mExitValue(-1),
+      mProcess(nsnull)
 {
-#if defined(PROCESSMODEL_WINAPI)
-    procInfo.dwProcessId = nsnull;
-#else
-    mProcess = nsnull;
-#endif
 }
 
 //Destructor
 nsProcess::~nsProcess()
 {
 #if defined(PROCESSMODEL_WINAPI)
-    if (procInfo.dwProcessId) {
-        CloseHandle( procInfo.hProcess );
-        CloseHandle( procInfo.hThread );
-    }
+    if (mProcess)
+        CloseHandle(mProcess);
 #else
     if (mProcess) 
         PR_DetachProcess(mProcess);
 #endif
 }
 
 NS_IMETHODIMP
 nsProcess::Init(nsIFile* executable)
 {
     //Prevent re-initializing if already attached to process
 #if defined(PROCESSMODEL_WINAPI)
-    if (procInfo.dwProcessId)
+    if (mProcess)
         return NS_ERROR_ALREADY_INITIALIZED;
 #else
     if (mProcess)
         return NS_ERROR_ALREADY_INITIALIZED;
 #endif    
 
     NS_ENSURE_ARG_POINTER(executable);
     PRBool isFile;
@@ -240,16 +233,17 @@ static int assembleCmdLine(char *const *
 #endif
 
 // XXXldb |args| has the wrong const-ness
 NS_IMETHODIMP  
 nsProcess::Run(PRBool blocking, const char **args, PRUint32 count)
 {
     NS_ENSURE_TRUE(mExecutable, NS_ERROR_NOT_INITIALIZED);
     PRStatus status = PR_SUCCESS;
+    mExitValue = -1;
 
     // make sure that when we allocate we have 1 greater than the
     // count since we need to null terminate the list for the argv to
     // pass into PR_CreateProcess
     char **my_argv = NULL;
     my_argv = (char **)nsMemory::Alloc(sizeof(char *) * (count + 2) );
     if (!my_argv) {
         return NS_ERROR_OUT_OF_MEMORY;
@@ -261,73 +255,75 @@ nsProcess::Run(PRBool blocking, const ch
         my_argv[i+1] = const_cast<char*>(args[i]);
     }
     // we need to set argv[0] to the program name.
     my_argv[0] = mTargetPath.BeginWriting();
     // null terminate the array
     my_argv[count+1] = NULL;
 
 #if defined(PROCESSMODEL_WINAPI)
-    STARTUPINFOW startupInfo;
     BOOL retVal;
     PRUnichar *cmdLine;
 
-    if (assembleCmdLine(my_argv, &cmdLine) == -1) {
+    if (count > 0 && assembleCmdLine(my_argv + 1, &cmdLine) == -1) {
         nsMemory::Free(my_argv);
         return NS_ERROR_FILE_EXECUTION_FAILED;    
     }
 
-    ZeroMemory(&startupInfo, sizeof(startupInfo));
-    startupInfo.cb = sizeof(startupInfo);
-
-    /* The CREATE_NO_WINDOW flag is important to prevent console
-     * windows from appearing.  This makes behavior the same on all
-     * platforms.  This won't work on win9x, however.  The flag will
-     * not have any effect on non-console applications.
+    /* The SEE_MASK_NO_CONSOLE flag is important to prevent console windows
+     * from appearing. This makes behavior the same on all platforms. The flag
+     * will not have any effect on non-console applications.
      */
+    PRInt32 numChars = MultiByteToWideChar(CP_ACP, 0, my_argv[0], -1, NULL, 0); 
+    PRUnichar* wideFile = (PRUnichar *) PR_MALLOC(numChars * sizeof(PRUnichar));
+    MultiByteToWideChar(CP_ACP, 0, my_argv[0], -1, wideFile, numChars); 
 
-    retVal = CreateProcessW(NULL,
-                            // const_cast<char*>(mTargetPath.get()),
-                            cmdLine,
-                            NULL,  /* security attributes for the new
-                                    * process */
-                            NULL,  /* security attributes for the primary
-                                    * thread in the new process */
-                            FALSE,  /* inherit handles */
-                            CREATE_NO_WINDOW, /* creation flags */
-                            NULL,  /* env */
-                            NULL,  /* current drive and directory */
-                            &startupInfo,
-                            &procInfo
-                           );
-    PR_Free( cmdLine );
+    SHELLEXECUTEINFOW sinfo;
+    memset(&sinfo, 0, sizeof(SHELLEXECUTEINFOW));
+    sinfo.cbSize = sizeof(SHELLEXECUTEINFOW);
+    sinfo.hwnd   = NULL;
+    sinfo.lpFile = wideFile;
+    sinfo.nShow  = SW_SHOWNORMAL;
+    sinfo.fMask  = SEE_MASK_FLAG_DDEWAIT |
+                   SEE_MASK_NO_CONSOLE |
+                   SEE_MASK_NOCLOSEPROCESS;
+
+    if (count > 0)
+        sinfo.lpParameters = cmdLine;
+
+    retVal = ShellExecuteExW(&sinfo);
+    mProcess = sinfo.hProcess;
+
+    PR_Free(wideFile);
+    if (count > 0)
+        PR_Free( cmdLine );
+
     if (blocking) {
- 
+
         // if success, wait for process termination. the early returns and such
         // are a bit ugly but preserving the logic of the nspr code I copied to 
         // minimize our risk abit.
 
         if ( retVal == TRUE ) {
             DWORD dwRetVal;
             unsigned long exitCode;
 
-            dwRetVal = WaitForSingleObject(procInfo.hProcess, INFINITE);
+            dwRetVal = WaitForSingleObject(mProcess, INFINITE);
             if (dwRetVal == WAIT_FAILED) {
                 nsMemory::Free(my_argv);
                 return NS_ERROR_FAILURE;
             }
-            if (GetExitCodeProcess(procInfo.hProcess, &exitCode) == FALSE) {
+            if (GetExitCodeProcess(mProcess, &exitCode) == FALSE) {
                 mExitValue = exitCode;
                 nsMemory::Free(my_argv);
-               return NS_ERROR_FAILURE;
+                return NS_ERROR_FAILURE;
             }
             mExitValue = exitCode;
-            CloseHandle(procInfo.hProcess);
-            CloseHandle(procInfo.hThread);
-            procInfo.dwProcessId = nsnull;
+            CloseHandle(mProcess);
+            mProcess = NULL;
         }
         else
             status = PR_FAILURE;
     } 
     else {
 
         // map return value into success code
 
@@ -356,25 +352,33 @@ nsProcess::Run(PRBool blocking, const ch
         return NS_ERROR_FILE_EXECUTION_FAILED;
 
     return NS_OK;
 }
 
 NS_IMETHODIMP nsProcess::GetIsRunning(PRBool *aIsRunning)
 {
 #if defined(PROCESSMODEL_WINAPI)
-    if (!procInfo.dwProcessId) {
+    if (!mProcess) {
         *aIsRunning = PR_FALSE;
         return NS_OK;
     }
     DWORD ec = 0;
-    BOOL br = GetExitCodeProcess(procInfo.hProcess, &ec);
+    BOOL br = GetExitCodeProcess(mProcess, &ec);
     if (!br)
         return NS_ERROR_FAILURE;
-    *aIsRunning = (ec == STILL_ACTIVE ? PR_TRUE : PR_FALSE); 
+    if (ec == STILL_ACTIVE) {
+        *aIsRunning = PR_TRUE;
+    }
+    else {
+        *aIsRunning = PR_FALSE;
+        mExitValue = ec;
+        CloseHandle(mProcess);
+        mProcess = NULL;
+    }
     return NS_OK;
 #elif defined WINCE
     return NS_ERROR_NOT_IMPLEMENTED;   
 #else
     if (!mProcess) {
         *aIsRunning = PR_FALSE;
         return NS_OK;
     }
@@ -397,19 +401,28 @@ nsProcess::GetLocation(nsIFile** aLocati
 {
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsProcess::GetPid(PRUint32 *aPid)
 {
 #if defined(PROCESSMODEL_WINAPI)
-    if (!procInfo.dwProcessId)
+    if (!mProcess)
         return NS_ERROR_FAILURE;
-    *aPid = procInfo.dwProcessId;
+    HMODULE kernelDLL = ::LoadLibraryW(L"kernel32.dll");
+    if (!kernelDLL)
+        return NS_ERROR_NOT_IMPLEMENTED;
+    GetProcessIdPtr getProcessId = (GetProcessIdPtr)GetProcAddress(kernelDLL, "GetProcessId");
+    if (!getProcessId) {
+        FreeLibrary(kernelDLL);
+        return NS_ERROR_NOT_IMPLEMENTED;
+    }
+    *aPid = getProcessId(mProcess);
+    FreeLibrary(kernelDLL);
     return NS_OK;
 #elif defined WINCE
     return NS_ERROR_NOT_IMPLEMENTED;
 #else
     if (!mProcess)
         return NS_ERROR_FAILURE;
 
     struct MYProcess {
@@ -432,36 +445,49 @@ nsProcess::GetProcessSignature(PRUint32 
 {
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsProcess::Kill()
 {
 #if defined(PROCESSMODEL_WINAPI)
-    if (!procInfo.dwProcessId)
+    if (!mProcess)
         return NS_ERROR_NOT_INITIALIZED;
 
-    if ( TerminateProcess(procInfo.hProcess, NULL) == 0 )
+    if ( TerminateProcess(mProcess, NULL) == 0 )
         return NS_ERROR_FAILURE;
 
-    CloseHandle( procInfo.hProcess );
-    CloseHandle( procInfo.hThread );
-    procInfo.dwProcessId = nsnull;
+    CloseHandle( mProcess );
+    mProcess = NULL;
 #else
     if (!mProcess)
         return NS_ERROR_NOT_INITIALIZED;
 
     if (PR_KillProcess(mProcess) != PR_SUCCESS)
         return NS_ERROR_FAILURE;
 
     mProcess = nsnull;
 #endif  
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsProcess::GetExitValue(PRInt32 *aExitValue)
 {
+#if defined(PROCESSMODEL_WINAPI)
+    if (mProcess) {
+        DWORD ec = 0;
+        BOOL br = GetExitCodeProcess(mProcess, &ec);
+        if (!br)
+            return NS_ERROR_FAILURE;
+        // If we have an exit code then the process has ended, clean it up.
+        if (ec != STILL_ACTIVE) {
+            mExitValue = ec;
+            CloseHandle(mProcess);
+            mProcess = NULL;
+        }
+    }
+#endif
     *aExitValue = mExitValue;
     
     return NS_OK;
 }