[reactos] 122/360: [WINESYNC] msi: Reuse the custom action server process where possible.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=82decec5f10d5c4bc70a06...
commit 82decec5f10d5c4bc70a0649b217fd1bfc49e060 Author: winesync ros-dev@reactos.org AuthorDate: Sat Mar 12 23:22:09 2022 +0100 Commit: Mark Jansen mark.jansen@reactos.org CommitDate: Sun Mar 20 19:27:57 2022 +0100
[WINESYNC] msi: Reuse the custom action server process where possible.
We use a named pipe to communicate between the client (i.e. the process that called MsiInstallProduct() and the custom action server. The naïve approach has the client send custom action GUIDs to the server and the server send back the results of executing the action, but this fails in the case of nested custom actions, so instead we send back handles of threads for the client to wait on.
Signed-off-by: Zebediah Figura z.figura12@gmail.com Signed-off-by: Hans Leidekker hans@codeweavers.com Signed-off-by: Alexandre Julliard julliard@winehq.org
wine commit id 457431ab5bc1dd10c4957f145de85c4ba6d0ef72 by Zebediah Figura z.figura12@gmail.com --- base/system/msiexec/msiexec.c | 59 ++++++++++--- dll/win32/msi/custom.c | 133 ++++++++++++++++++++++------- dll/win32/msi/msipriv.h | 6 ++ dll/win32/msi/package.c | 4 + modules/rostests/winetests/msi/custom.c | 19 +++++ modules/rostests/winetests/msi/custom.spec | 3 + modules/rostests/winetests/msi/install.c | 14 ++- 7 files changed, 198 insertions(+), 40 deletions(-)
diff --git a/base/system/msiexec/msiexec.c b/base/system/msiexec/msiexec.c index 74f6d515b95..5a471a21840 100644 --- a/base/system/msiexec/msiexec.c +++ b/base/system/msiexec/msiexec.c @@ -30,6 +30,9 @@ #include "wine/debug.h" #include "wine/unicode.h"
+#include "initguid.h" +DEFINE_GUID(GUID_NULL,0,0,0,0,0,0,0,0,0,0,0); + WINE_DEFAULT_DEBUG_CHANNEL(msiexec);
typedef HRESULT (WINAPI *DLLREGISTERSERVER)(void); @@ -401,24 +404,60 @@ static DWORD CALLBACK custom_action_thread(void *arg) return __wine_msi_call_dll_function(guid); }
-static int DoEmbedding(LPCWSTR key) +static int custom_action_server(const WCHAR *arg) { + static const WCHAR pipe_name[] = {'\','\','.','\','p','i','p','e','\','m','s','i','c','a','_','%','x',0}; + DWORD client_pid = atoiW(arg); + DWORD64 thread64; + WCHAR buffer[24]; HANDLE thread; + HANDLE pipe; + DWORD size; GUID guid; - UINT r; + + TRACE("%s\n", debugstr_w(arg)); + + if (!client_pid) + { + ERR("Invalid parameter %s\n", debugstr_w(arg)); + return 1; + } + + sprintfW(buffer, pipe_name, client_pid); + pipe = CreateFileW(buffer, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL); + if (pipe == INVALID_HANDLE_VALUE) + { + ERR("Failed to create custom action server pipe: %u\n", GetLastError()); + return GetLastError(); + }
/* We need this to unmarshal streams, and some apps expect it to be present. */ CoInitializeEx(NULL, COINIT_MULTITHREADED);
- CLSIDFromString(key, &guid); - thread = CreateThread(NULL, 0, custom_action_thread, &guid, 0, NULL); - WaitForSingleObject(thread, INFINITE); - GetExitCodeThread(thread, &r); - CloseHandle(thread); + while (ReadFile(pipe, &guid, sizeof(guid), &size, NULL) && size == sizeof(guid)) + { + if (IsEqualGUID(&guid, &GUID_NULL)) + { + /* package closed; time to shut down */ + CoUninitialize(); + return 0; + }
- CoUninitialize(); + thread = CreateThread(NULL, 0, custom_action_thread, &guid, 0, NULL);
- return r; + /* give the thread handle to the client to wait on, since we might have + * to run a nested action and can't block during this one */ + thread64 = (DWORD_PTR)thread; + if (!WriteFile(pipe, &thread64, sizeof(thread64), &size, NULL) || size != sizeof(thread64)) + { + ERR("Failed to write to custom action server pipe: %u\n", GetLastError()); + CoUninitialize(); + return GetLastError(); + } + } + ERR("Failed to read from custom action server pipe: %u\n", GetLastError()); + CoUninitialize(); + return GetLastError(); }
/* @@ -615,7 +654,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine }
if (argc == 3 && msi_option_equal(argvW[1], "Embedding")) - return DoEmbedding( argvW[2] ); + return custom_action_server(argvW[2]);
for(i = 1; i < argc; i++) { diff --git a/dll/win32/msi/custom.c b/dll/win32/msi/custom.c index 4202f255973..e7e19a2475a 100644 --- a/dll/win32/msi/custom.c +++ b/dll/win32/msi/custom.c @@ -603,22 +603,88 @@ UINT CDECL __wine_msi_call_dll_function(const GUID *guid) return r; }
-static DWORD WINAPI custom_client_thread(void *arg) +static DWORD custom_start_server(MSIPACKAGE *package, DWORD arch) { + static const WCHAR pipe_name[] = {'\','\','.','\','p','i','p','e','\','m','s','i','c','a','_','%','x',0}; static const WCHAR msiexecW[] = {'\','m','s','i','e','x','e','c','.','e','x','e',0}; - static const WCHAR argsW[] = {' ','-','E','m','b','e','d','d','i','n','g',' ',0}; - msi_custom_action_info *info = arg; + static const WCHAR argsW[] = {'%','s',' ','-','E','m','b','e','d','d','i','n','g',' ','%','d',0}; + + WCHAR path[MAX_PATH], cmdline[MAX_PATH + 23]; PROCESS_INFORMATION pi = {0}; STARTUPINFOW si = {0}; - WCHAR buffer[MAX_PATH], cmdline[MAX_PATH + 60]; - RPC_STATUS status; + WCHAR buffer[24]; void *cookie; + HANDLE pipe; BOOL wow64; + + if ((arch == SCS_32BIT_BINARY && package->custom_server_32_process) || + (arch == SCS_64BIT_BINARY && package->custom_server_64_process)) + return ERROR_SUCCESS; + + sprintfW(buffer, pipe_name, GetCurrentProcessId()); + pipe = CreateNamedPipeW(buffer, PIPE_ACCESS_DUPLEX, 0, 1, sizeof(DWORD64), + sizeof(GUID), 0, NULL); + + if (sizeof(void *) == 8 && arch == SCS_32BIT_BINARY) + GetSystemWow64DirectoryW(path, MAX_PATH - sizeof(msiexecW)/sizeof(WCHAR)); + else + GetSystemDirectoryW(path, MAX_PATH - sizeof(msiexecW)/sizeof(WCHAR)); + strcatW(path, msiexecW); + sprintfW(cmdline, argsW, path, GetCurrentProcessId()); + + if (IsWow64Process(GetCurrentProcess(), &wow64) && wow64 && arch == SCS_64BIT_BINARY) + { + Wow64DisableWow64FsRedirection(&cookie); + CreateProcessW(path, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + Wow64RevertWow64FsRedirection(cookie); + } + else + CreateProcessW(path, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + + CloseHandle(pi.hThread); + + if (arch == SCS_32BIT_BINARY) + { + package->custom_server_32_process = pi.hProcess; + package->custom_server_32_pipe = pipe; + } + else + { + package->custom_server_64_process = pi.hProcess; + package->custom_server_64_pipe = pipe; + } + + if (!ConnectNamedPipe(pipe, NULL)) + { + ERR("Failed to connect to custom action server: %u\n", GetLastError()); + return GetLastError(); + } + + return ERROR_SUCCESS; +} + +void custom_stop_server(HANDLE process, HANDLE pipe) +{ + DWORD size; + + WriteFile(pipe, &GUID_NULL, sizeof(GUID_NULL), &size, NULL); + WaitForSingleObject(process, INFINITE); + CloseHandle(process); + CloseHandle(pipe); +} + +static DWORD WINAPI custom_client_thread(void *arg) +{ + msi_custom_action_info *info = arg; + RPC_STATUS status; + DWORD64 thread64; + HANDLE process; + HANDLE thread; + HANDLE pipe; DWORD arch; + DWORD size; BOOL ret; - DWORD rc; - - TRACE("custom action (%x) started\n", GetCurrentThreadId() ); + UINT rc;
CoInitializeEx(NULL, COINIT_MULTITHREADED); /* needed to marshal streams */
@@ -644,35 +710,44 @@ static DWORD WINAPI custom_client_thread(void *arg) }
ret = GetBinaryTypeW(info->source, &arch); + if (!ret) + arch = (sizeof(void *) == 8 ? SCS_64BIT_BINARY : SCS_32BIT_BINARY);
- if (sizeof(void *) == 8 && ret && arch == SCS_32BIT_BINARY) - GetSystemWow64DirectoryW(buffer, MAX_PATH - sizeof(msiexecW)/sizeof(WCHAR)); + custom_start_server(info->package, arch); + if (arch == SCS_32BIT_BINARY) + { + process = info->package->custom_server_32_process; + pipe = info->package->custom_server_32_pipe; + } else - GetSystemDirectoryW(buffer, MAX_PATH - sizeof(msiexecW)/sizeof(WCHAR)); - strcatW(buffer, msiexecW); - strcpyW(cmdline, buffer); - strcatW(cmdline, argsW); - StringFromGUID2(&info->guid, cmdline + strlenW(cmdline), 39); + { + process = info->package->custom_server_64_process; + pipe = info->package->custom_server_64_pipe; + }
- if (IsWow64Process(GetCurrentProcess(), &wow64) && wow64 && arch == SCS_64BIT_BINARY) + if (!WriteFile(pipe, &info->guid, sizeof(info->guid), &size, NULL) || + size != sizeof(info->guid)) { - Wow64DisableWow64FsRedirection(&cookie); - CreateProcessW(buffer, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); - Wow64RevertWow64FsRedirection(cookie); + ERR("Failed to write to custom action client pipe: %u\n", GetLastError()); + return GetLastError(); + } + if (!ReadFile(pipe, &thread64, sizeof(thread64), &size, NULL) || size != sizeof(thread64)) + { + ERR("Failed to read from custom action client pipe: %u\n", GetLastError()); + return GetLastError(); } - else - CreateProcessW(buffer, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
- WaitForSingleObject(pi.hProcess, INFINITE); - GetExitCodeProcess(pi.hProcess, &rc); - CloseHandle(pi.hProcess); - CloseHandle(pi.hThread); + if (DuplicateHandle(process, (HANDLE)(DWORD_PTR)thread64, GetCurrentProcess(), + &thread, 0, FALSE, DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) + { + WaitForSingleObject(thread, INFINITE); + GetExitCodeThread(thread, &rc); + CloseHandle(thread); + } + else + rc = GetLastError();
CoUninitialize(); - - TRACE("custom action (%x) returned %i\n", GetCurrentThreadId(), rc ); - - MsiCloseAllHandles(); return rc; }
diff --git a/dll/win32/msi/msipriv.h b/dll/win32/msi/msipriv.h index f701f91a866..7a7da867e2a 100644 --- a/dll/win32/msi/msipriv.h +++ b/dll/win32/msi/msipriv.h @@ -424,6 +424,11 @@ typedef struct tagMSIPACKAGE
struct list RunningActions;
+ HANDLE custom_server_32_process; + HANDLE custom_server_64_process; + HANDLE custom_server_32_pipe; + HANDLE custom_server_64_pipe; + LPWSTR PackagePath; LPWSTR ProductCode; LPWSTR localfile; @@ -983,6 +988,7 @@ extern HINSTANCE msi_hInstance DECLSPEC_HIDDEN; extern UINT ACTION_PerformAction(MSIPACKAGE *package, const WCHAR *action) DECLSPEC_HIDDEN; extern void ACTION_FinishCustomActions( const MSIPACKAGE* package) DECLSPEC_HIDDEN; extern UINT ACTION_CustomAction(MSIPACKAGE *package, const WCHAR *action) DECLSPEC_HIDDEN; +extern void custom_stop_server(HANDLE process, HANDLE pipe) DECLSPEC_HIDDEN;
/* actions in other modules */ extern UINT ACTION_AppSearch(MSIPACKAGE *package) DECLSPEC_HIDDEN; diff --git a/dll/win32/msi/package.c b/dll/win32/msi/package.c index 03927046190..577fcbbf185 100644 --- a/dll/win32/msi/package.c +++ b/dll/win32/msi/package.c @@ -346,6 +346,10 @@ static void MSI_FreePackage( MSIOBJECTHDR *arg) RpcServerUnregisterIf(s_IWineMsiRemote_v0_0_s_ifspec, NULL, FALSE); if (rpc_handle) RpcBindingFree(&rpc_handle); + if (package->custom_server_32_process) + custom_stop_server(package->custom_server_32_process, package->custom_server_32_pipe); + if (package->custom_server_64_process) + custom_stop_server(package->custom_server_64_process, package->custom_server_64_pipe);
if (package->delete_on_close) DeleteFileW( package->localfile ); msi_free( package->localfile ); diff --git a/modules/rostests/winetests/msi/custom.c b/modules/rostests/winetests/msi/custom.c index 4d45c9e448f..59a69ee22f8 100644 --- a/modules/rostests/winetests/msi/custom.c +++ b/modules/rostests/winetests/msi/custom.c @@ -1150,6 +1150,25 @@ UINT WINAPI da_deferred(MSIHANDLE hinst) return ERROR_SUCCESS; }
+static int global_state; + +UINT WINAPI process1(MSIHANDLE hinst) +{ + SetEnvironmentVariableA("MSI_PROCESS_TEST","1"); + global_state++; + return ERROR_SUCCESS; +} + +UINT WINAPI process2(MSIHANDLE hinst) +{ + char env[2] = {0}; + DWORD r = GetEnvironmentVariableA("MSI_PROCESS_TEST", env, sizeof(env)); + ok(hinst, r == 1, "got %d, error %u\n", r, GetLastError()); + ok(hinst, !strcmp(env, "1"), "got %s\n", env); + ok(hinst, !global_state, "got global_state %d\n", global_state); + return ERROR_SUCCESS; +} + static BOOL pf_exists(const char *file) { char path[MAX_PATH]; diff --git a/modules/rostests/winetests/msi/custom.spec b/modules/rostests/winetests/msi/custom.spec index b94f6cce703..b7cbe582506 100644 --- a/modules/rostests/winetests/msi/custom.spec +++ b/modules/rostests/winetests/msi/custom.spec @@ -3,6 +3,9 @@ @ stdcall da_immediate(long) @ stdcall da_deferred(long) @ stdcall nested(long) +@ stdcall process1(long) +@ stdcall process2(long) + @ stdcall cf_present(long) @ stdcall cf_absent(long) @ stdcall crs_present(long) diff --git a/modules/rostests/winetests/msi/install.c b/modules/rostests/winetests/msi/install.c index f21e32789f3..80137c1d993 100644 --- a/modules/rostests/winetests/msi/install.c +++ b/modules/rostests/winetests/msi/install.c @@ -670,9 +670,14 @@ static const CHAR ca1_install_exec_seq_dat[] = "Action\tCondition\tSequence\n" "FileCost\t\t200\n" "CostFinalize\t\t300\n" "InstallValidate\t\t400\n" + "InstallInitialize\t\t500\n" "embednull\t\t600\n" "maintest\tMAIN_TEST\t700\n" - "testretval\tTEST_RETVAL\t710\n"; + "testretval\tTEST_RETVAL\t710\n" + "process1\tTEST_PROCESS\t720\n" + "process2\tTEST_PROCESS\t721\n" + "process_deferred\tTEST_PROCESS\t722\n" + "InstallFinalize\t\t800\n";
static const CHAR ca1_custom_action_dat[] = "Action\tType\tSource\tTarget\n" "s72\ti2\tS64\tS0\n" @@ -681,6 +686,9 @@ static const CHAR ca1_custom_action_dat[] = "Action\tType\tSource\tTarget\n" "nested51\t51\tnested\t1\n" "nested1\t1\tcustom.dll\tnested\n" "maintest\t1\tcustom.dll\tmain_test\n" + "process1\t1\tcustom.dll\tprocess1\n" + "process2\t1\tcustom.dll\tprocess2\n" + "process_deferred\t1025\tcustom.dll\tprocess2\n" "testretval\t1\tcustom.dll\ttest_retval\n";
static const CHAR ca1_test_seq_dat[] = "Action\tCondition\tSequence\n" @@ -4134,6 +4142,10 @@ static void test_customaction1(void) r = MsiInstallProductA(msifile, "TEST_RETVAL=1"); ok(r == ERROR_INSTALL_FAILURE, "Expected ERROR_INSTALL_FAILURE, got %u\n", r);
+ /* Custom actions execute in the same process, but they don't retain state */ + r = MsiInstallProductA(msifile, "TEST_PROCESS=1"); + ok(!r, "got %u\n", r); + delete_test_files(); DeleteFileA(msifile); DeleteFileA("unus");
-
winesync