fix(windows): start keymanx64 with ShellExecute

Fixes #5196.

It is complicated to start a `UIAccess=true` process with
`CreateProcess`, so instead we'll use `ShellExecute`. This means passing
a PID instead of a process handle to keymanx64.

I took the opportunity to refactor slightly the main function in
keymanx64, as the failure modes were pretty WET.

windows/src/engine/keyman/UfrmKeyman7Main.pas

@@ -1948,36 +1948,33 @@ end;
 
 procedure TfrmKeyman7Main.StartKeymanX64;
 var
-  cmd: string;
-  si: TStartupInfoW;
-  pi: TProcessInformation;
-  v: PWideChar;
-  processHandle: THandle;
+  cmd, dir, params: string;
+  sei: TShellExecuteInfoW;
 begin
   if not IsWow64 then Exit;   // I4374
 
   try
-    FillChar(si, sizeof(TStartupInfoW), 0);
-    FillChar(pi, sizeof(TProcessInformation), 0);
-    si.cb := sizeof(TStartupInfo);
-    si.dwFlags := STARTF_USESHOWWINDOW;
-    si.wShowWindow := SW_HIDE;
+    // TODO: use TKeymanPaths to find keymanx64?
+    dir := ExtractFilePath(ParamStr(0));
+    cmd := dir + 'keymanx64.exe';
+    params := Format('%d %d', [GetCurrentProcessId, Application.Handle]);
 
-    if not DuplicateHandle(GetCurrentProcess, GetCurrentProcess,
-        GetCurrentProcess, @processHandle, 0, True, DUPLICATE_SAME_ACCESS) then
+    if not FileExists(cmd) then
+      // We'll get notification of the issue but it won't
+      // crash the process
+      raise Exception.Create(cmd+' could not be found');
+
+    FillChar(sei, SizeOf(sei), 0);
+    sei.cbSize := SizeOf(sei);
+    sei.Wnd := Handle;
+    sei.lpVerb := 'open';
+    sei.lpFile := PWideChar(cmd);
+    sei.lpParameters := PChar(params);
+    sei.lpDirectory := PWideChar(dir);
+    sei.nShow := SW_SHOW;
+
+    if not ShellExecuteExW(@sei) then
       RaiseLastOSError;
-    cmd := Format('%s%s %d %d', [ExtractFilePath(ParamStr(0)), 'keymanx64.exe', processHandle, Application.Handle]);
-    // Duplicate the string because CreateProcess requires a mutable buffer, so
-    // this guarantees it
-    v := StrNew(PWideChar(cmd));
-    try
-      if not CreateProcessW(nil, v, nil, nil, True, NORMAL_PRIORITY_CLASS, nil, nil, si, pi) then
-        RaiseLastOSError;
-      CloseHandle(pi.hProcess);
-      CloseHandle(pi.hThread);
-    finally
-      StrDispose(v);
-    end;
   except
     on E:Exception do
     begin

windows/src/engine/keymanx64/keymanx64.cpp

@@ -26,6 +26,7 @@
 
 // Forward declarations of functions included in this code module
 
+BOOL             Run(HINSTANCE hPrevInstance, LPTSTR lpCmdLine, int nCmdShow);
 ATOM             MyRegisterClass(HINSTANCE hInstance);
 BOOL             InitInstance(HINSTANCE, int);
 LRESULT CALLBACK WndProc(HWND, UINT, WPARAM, LPARAM);
@@ -53,6 +54,7 @@ HINSTANCE hInst;                            // Current instance
 HWND hwndController = NULL;                 // Keyman x86 Application window handle
 DWORD dwControllerThreadId = NULL;
 HANDLE hParentProcessHandle = NULL;         // Keyman x86 process handle
+DWORD dwParentProcessID = 0;
 
 HANDLE hWatcherThread = NULL;
 HANDLE hWatcherThreadTerminateEvent = NULL;
@@ -70,6 +72,7 @@ const PWSTR
   szError_Keymanx86NotFound_Comms = L"Keyman Engine x86 has closed unexpectedly.  Closing down Keyman Engine x64",
   szError_FailedToRegisterController = L"Failed to register controller window",
   szError_FailedToInitialise = L"Failed to initialise Keyman Engine x64",
+  szError_FailedToFindParentProcess = L"Unable to open parent process; it may have disappeared",
 
   szError_ChangeWindowMessageFilter = L"Failed to prepare message filters",
 
@@ -117,18 +120,38 @@ int APIENTRY _tWinMain(HINSTANCE hInstance,
     keyman_sentry_test_crash();
   }
 
+  BOOL result = Run(hInstance, lpCmdLine, nCmdShow);
+
+  keyman_sentry_shutdown();
+
+  return result ? 0 : 1;
+}
+
+BOOL Run(HINSTANCE hInstance,
+         LPTSTR    lpCmdLine,
+         int       nCmdShow) {
 	MSG msg;
 
   if (!ParseCmdLine(lpCmdLine)) {
-    Fail(0, szError_InvalidCommandline);
-    keyman_sentry_shutdown();
-    return 1;
+    return Fail(0, szError_InvalidCommandline);
+  }
+
+  // We pass the parent process ID rather than a handle because it is much
+  // easier to instantiate a UIAccess=true process with ShellExecuteEx than
+  // with CreateProcess. However, ShellExecuteEx does not allow us to
+  // inherit handles, so we'll just open the parent process as soon as we
+  // can after loading. There is a small chance of the parent process
+  // disappearing before the handle can be opened, in which case it is in
+  // theory possible that this could be waiting on the wrong process. We have
+  // reduced this chance of this happening by comparing the hwndController
+  // PID with the parent PID (see ParseCmdLine).
+  hParentProcessHandle = OpenProcess(SYNCHRONIZE, FALSE, dwParentProcessID);
+  if (hParentProcessHandle == NULL) {
+    return Fail(0, szError_FailedToFindParentProcess);
   }
 
   if (!MyRegisterClass(hInstance)) {
-    Fail(0, szError_FailedToRegister);
-    keyman_sentry_shutdown();
-    return 1;
+    return Fail(0, szError_FailedToRegister);
   }
 
   if (!InitInstance(hInstance, nCmdShow)) {
@@ -141,8 +164,7 @@ int APIENTRY _tWinMain(HINSTANCE hInstance,
       // be reporting it.
       Fail(0, szError_FailedToInitInstance);
     }
-    keyman_sentry_shutdown();
-    return 1;
+    return FALSE;
   }
 
 	// Main message loop:
@@ -152,9 +174,9 @@ int APIENTRY _tWinMain(HINSTANCE hInstance,
 		DispatchMessage(&msg);
 	}
 
-  keyman_sentry_shutdown();
-
-	return (int) msg.wParam;
+  // If msg.wParam is non-zero, there was a
+  // failure returned from the main window
+	return msg.wParam ? FALSE : TRUE;
 }
 
 //
@@ -170,15 +192,17 @@ int APIENTRY _tWinMain(HINSTANCE hInstance,
 BOOL ParseCmdLine(LPTSTR lpCmdLine) {
   while (iswspace(*lpCmdLine)) lpCmdLine++;
   if (!*lpCmdLine) return FALSE;
-  hParentProcessHandle = (HANDLE)wcstoull(lpCmdLine, &lpCmdLine, 10);
-  if (hParentProcessHandle == 0 || hParentProcessHandle == (HANDLE) ULLONG_MAX) return FALSE;
+  dwParentProcessID = (DWORD) wcstoul(lpCmdLine, &lpCmdLine, 10);
+  if (dwParentProcessID == 0 || dwParentProcessID == ULONG_MAX) return FALSE;
 
   while (iswspace(*lpCmdLine)) lpCmdLine++;
   if (!*lpCmdLine) return FALSE;
   hwndController = (HWND) wcstoull(lpCmdLine, &lpCmdLine, 10);
   if (hwndController == 0 || hwndController == (HWND) ULLONG_MAX) return FALSE;
-  dwControllerThreadId = GetWindowThreadProcessId(hwndController, NULL);
-  if (dwControllerThreadId == NULL) return FALSE;
+
+  DWORD dwControllerProcessID;
+  dwControllerThreadId = GetWindowThreadProcessId(hwndController, &dwControllerProcessID);
+  if (dwControllerThreadId == NULL || dwControllerProcessID != dwParentProcessID) return FALSE;
 
   return TRUE;
 }