[PATCH 4/4] msi: Reuse the custom action server process where possible.
Zebediah Figura
z.figura12 at gmail.com
Tue Jul 10 05:08:47 CDT 2018
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 at gmail.com>
---
dlls/msi/custom.c | 137 +++++++++++++++++++++++++++++++++++----------
dlls/msi/msipriv.h | 6 ++
dlls/msi/package.c | 4 ++
dlls/msi/tests/custom.c | 19 +++++++
dlls/msi/tests/custom.spec | 3 +
dlls/msi/tests/install.c | 14 ++++-
programs/msiexec/msiexec.c | 59 +++++++++++++++----
7 files changed, 200 insertions(+), 42 deletions(-)
diff --git a/dlls/msi/custom.c b/dlls/msi/custom.c
index 0fa47bc..66f9dca 100644
--- a/dlls/msi/custom.c
+++ b/dlls/msi/custom.c
@@ -588,22 +588,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;
- DWORD arch;
- BOOL ret;
- DWORD rc;
- TRACE("custom action (%x) started\n", GetCurrentThreadId() );
+ 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;
+ UINT rc;
CoInitializeEx(NULL, COINIT_MULTITHREADED); /* needed to marshal streams */
@@ -629,35 +695,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));
- 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);
-
- if (IsWow64Process(GetCurrentProcess(), &wow64) && wow64 && arch == SCS_64BIT_BINARY)
+ custom_start_server(info->package, arch);
+ if (arch == SCS_32BIT_BINARY)
{
- Wow64DisableWow64FsRedirection(&cookie);
- CreateProcessW(buffer, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
- Wow64RevertWow64FsRedirection(cookie);
+ process = info->package->custom_server_32_process;
+ pipe = info->package->custom_server_32_pipe;
}
else
- CreateProcessW(buffer, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
+ {
+ process = info->package->custom_server_64_process;
+ pipe = info->package->custom_server_64_pipe;
+ }
- WaitForSingleObject(pi.hProcess, INFINITE);
- GetExitCodeProcess(pi.hProcess, &rc);
- CloseHandle(pi.hProcess);
- CloseHandle(pi.hThread);
+ if (!WriteFile(pipe, &info->guid, sizeof(info->guid), &size, NULL) ||
+ size != sizeof(info->guid))
+ {
+ 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();
+ }
+
+ 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/dlls/msi/msipriv.h b/dlls/msi/msipriv.h
index 060fd7e..34bc490 100644
--- a/dlls/msi/msipriv.h
+++ b/dlls/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;
@@ -982,6 +987,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/dlls/msi/package.c b/dlls/msi/package.c
index 1ab52ea..0d937d6 100644
--- a/dlls/msi/package.c
+++ b/dlls/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/dlls/msi/tests/custom.c b/dlls/msi/tests/custom.c
index cf1289e..3989ca8 100644
--- a/dlls/msi/tests/custom.c
+++ b/dlls/msi/tests/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/dlls/msi/tests/custom.spec b/dlls/msi/tests/custom.spec
index b94f6cc..b7cbe58 100644
--- a/dlls/msi/tests/custom.spec
+++ b/dlls/msi/tests/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/dlls/msi/tests/install.c b/dlls/msi/tests/install.c
index 663a27b..e147df6 100644
--- a/dlls/msi/tests/install.c
+++ b/dlls/msi/tests/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");
diff --git a/programs/msiexec/msiexec.c b/programs/msiexec/msiexec.c
index 74f6d51..5a471a2 100644
--- a/programs/msiexec/msiexec.c
+++ b/programs/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;
+ }
+ thread = CreateThread(NULL, 0, custom_action_thread, &guid, 0, NULL);
+
+ /* 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 r;
+ 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++)
{
--
2.7.4
More information about the wine-devel
mailing list