Francois Gouget : testbot/testagentd: Fix the upgrade RPC.
Alexandre Julliard
julliard at winehq.org
Tue Mar 27 16:08:19 CDT 2018
Module: tools
Branch: master
Commit: 1a1a852cec0f714286c2801c8902308a95a8dab3
URL: https://source.winehq.org/git/tools.git/?a=commit;h=1a1a852cec0f714286c2801c8902308a95a8dab3
Author: Francois Gouget <fgouget at codeweavers.com>
Date: Tue Mar 27 03:44:36 2018 +0200
testbot/testagentd: Fix the upgrade RPC.
On Windows it used to work only once and leave the batch script behind.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>
---
testbot/src/testagentd/platform.h | 9 ++-
testbot/src/testagentd/platform_unix.c | 62 +++++++++++++++------
testbot/src/testagentd/platform_windows.c | 92 ++++++++++++++++++++++++-------
testbot/src/testagentd/testagentd.c | 31 ++++-------
4 files changed, 130 insertions(+), 64 deletions(-)
diff --git a/testbot/src/testagentd/platform.h b/testbot/src/testagentd/platform.h
index 98dfe39..06882de 100644
--- a/testbot/src/testagentd/platform.h
+++ b/testbot/src/testagentd/platform.h
@@ -96,12 +96,11 @@ int platform_rmchildproc(SOCKET client, uint64_t pid);
*/
int platform_settime(uint64_t epoch, uint32_t leeway);
-/* Creates a script to be invoked to upgrade the current server.
- * The current server is responsible for starting the script and quickly exit.
- * The script will wait a bit, replace the server file and restart the server
- * with the same arguments as the original server.
+/* Replaces server file and restarts the server.
+ * Returns non-zero if successful, which will let the caller know to complete
+ * the current RPC and cleanly exit. Returns 0 otherwise.
*/
-int platform_upgrade_script(const char* script, const char* tmpserver, char** argv);
+int platform_upgrade(const char* tmpserver, char** argv);
/* Called when the message has been dismissed by the user.
*/
diff --git a/testbot/src/testagentd/platform_unix.c b/testbot/src/testagentd/platform_unix.c
index fbaa995..7a3a8f8 100644
--- a/testbot/src/testagentd/platform_unix.c
+++ b/testbot/src/testagentd/platform_unix.c
@@ -239,34 +239,60 @@ int platform_settime(uint64_t epoch, uint32_t leeway)
return 1;
}
-int platform_upgrade_script(const char* script, const char* tmpserver, char** argv)
+
+int platform_upgrade(const char* tmpserver, char** argv)
{
- char** arg;
- FILE* fh;
+ static const char* oldserver = "testagentd.old";
+ int pipefds[2];
+ pid_t pid;
- fh = fopen(script, "w");
- if (!fh)
+ if (rename(argv[0], oldserver))
{
- set_status(ST_ERROR, "unable to open '%s' for writing: %s", script, strerror(errno));
+ set_status(ST_ERROR, "unable to move the current server file out of the way: %s", strerror(errno));
return 0;
}
- /* Allow time for the server to exit */
- fprintf(fh, "#!/bin/sh\n");
- fprintf(fh, "sleep 1\n");
- fprintf(fh, "mv %s %s\n", tmpserver, argv[0]);
- arg = argv;
- while (*arg)
+
+ if (rename(tmpserver, argv[0]))
{
- fprintf(fh, "'%s' ", *arg);
- arg++;
+ set_status(ST_ERROR, "unable to move the currentnew server file into place: %s", strerror(errno));
+ rename(oldserver, argv[0]);
+ return 0;
}
- fprintf(fh, "\n");
- fclose(fh);
- if (chmod(script, S_IRUSR | S_IWUSR | S_IXUSR) < 0)
+ if (pipe(pipefds))
{
- set_status(ST_ERROR, "could not make '%s' executable: %s", script, strerror(errno));
+ set_status(ST_ERROR, "could not synchronize with the new process: %s", strerror(errno));
+ rename(oldserver, argv[0]);
return 0;
}
+
+ pid = fork();
+ if (pid < 0)
+ {
+ set_status(ST_ERROR, "unable to start the new server: %s", strerror(errno));
+ close(pipefds[0]);
+ close(pipefds[1]);
+ rename(oldserver, argv[0]);
+ return 0;
+ }
+
+ unlink(oldserver);
+ if (!pid)
+ {
+ /* The child process is responsible for cleanly closing the connection
+ * to the client.
+ */
+ close(pipefds[0]);
+ return 1;
+ }
+ close(pipefds[1]);
+
+ /* Wait for the read to fail which means the child exited and released the
+ * TestAgentd port.
+ */
+ read(pipefds[0], &pid, 1);
+ close(pipefds[0]);
+
+ execvp(argv[0], argv);
return 1;
}
diff --git a/testbot/src/testagentd/platform_windows.c b/testbot/src/testagentd/platform_windows.c
index 5c55b20..bb52a02 100644
--- a/testbot/src/testagentd/platform_windows.c
+++ b/testbot/src/testagentd/platform_windows.c
@@ -280,36 +280,68 @@ int platform_settime(uint64_t epoch, uint32_t leeway)
}
-int platform_upgrade_script(const char* script, const char* tmpserver, char** argv)
+char* get_server_filename(int old)
{
- char testagentd[MAX_PATH];
+ char filename[MAX_PATH];
DWORD rc;
- FILE* fh;
- rc = GetModuleFileName(NULL, testagentd, sizeof(testagentd));
- if (!rc || rc == sizeof(testagentd))
+ rc = GetModuleFileName(NULL, filename, sizeof(filename));
+ if (!rc || rc == sizeof(filename))
+ return NULL;
+
+ if (old)
{
- set_status(ST_ERROR, "unable to get the current process filename (%lu, le=%lu)", rc, GetLastError());
- return 0;
+ if (rc >= sizeof(filename) - 5)
+ return NULL;
+ strcat(filename, ".old");
}
+ return strdup(filename);
+}
+
+int platform_upgrade(const char* tmpserver, char** argv)
+{
+ char *testagentd = NULL, *oldtestagentd = NULL;
+ STARTUPINFO si;
+ PROCESS_INFORMATION pi;
+ int success = 0;
- fh = fopen(script, "w");
- if (!fh)
+ testagentd = get_server_filename(0);
+ oldtestagentd = get_server_filename(1);
+ if (!testagentd || !oldtestagentd)
{
- set_status(ST_ERROR, "unable to open '%s' for writing: %s", script, strerror(errno));
- return 0;
+ set_status(ST_ERROR, "unable to get the process filenames (le=%lu)", GetLastError());
+ goto done;
}
- /* Allow time for the server to exit */
- fprintf(fh, "ping -n 1 -w 1000 1.1.1.1 >/nul\r\n");
- /* Note that preserving the server filename is necessary and sufficient
- * in order to get through the Windows firewall.
- */
- fprintf(fh, "copy /y \"%s\" \"%s\"\r\n", tmpserver, testagentd);
- fprintf(fh, "del \"%s\"\r\n", tmpserver);
- fprintf(fh, "%s\r\n", GetCommandLine());
- fclose(fh);
- return 1;
+ if (!MoveFile(testagentd, oldtestagentd))
+ {
+ set_status(ST_ERROR, "unable to move the current server file out of the way (le=%lu)", GetLastError());
+ goto done;
+ }
+ if (!MoveFile(tmpserver, testagentd))
+ {
+ set_status(ST_ERROR, "unable to move the new server file in place (le=%lu)", GetLastError());
+ MoveFile(oldtestagentd, testagentd);
+ goto done;
+ }
+
+ memset(&si, 0, sizeof(si));
+ si.cb = sizeof(si);
+ if (!CreateProcessA(testagentd, GetCommandLineA(), NULL, NULL, TRUE,
+ CREATE_NEW_CONSOLE, NULL, NULL, &si, &pi))
+ {
+ set_status(ST_ERROR, "could not run '%s': %lu", GetCommandLineA(), GetLastError());
+ MoveFile(oldtestagentd, testagentd);
+ goto done;
+ }
+
+ /* The new server will delete the old server file on startup */
+ success = 1;
+
+ done:
+ free(testagentd);
+ free(oldtestagentd);
+ return success;
}
struct msg_thread_t
@@ -488,11 +520,29 @@ void platform_detach_console(void)
int platform_init(void)
{
+ char *oldtestagentd;
HMODULE hdll;
WORD wVersionRequested;
WSADATA wsaData;
int rc;
+ /* Delete the old server file if any */
+ oldtestagentd = get_server_filename(1);
+ if (oldtestagentd)
+ {
+ /* This also serves to ensure the old server has released the port
+ * before we attempt to open our own.
+ */
+ do
+ {
+ if (!DeleteFileA(oldtestagentd))
+ Sleep(500);
+ }
+ while (GetLastError() == ERROR_ACCESS_DENIED);
+ free(oldtestagentd);
+ }
+
+
wVersionRequested = MAKEWORD(2, 2);
rc = WSAStartup(wVersionRequested, &wsaData);
if (rc)
diff --git a/testbot/src/testagentd/testagentd.c b/testbot/src/testagentd/testagentd.c
index 79016c0..b4a26fc 100644
--- a/testbot/src/testagentd/testagentd.c
+++ b/testbot/src/testagentd/testagentd.c
@@ -1078,7 +1078,6 @@ static void do_setproperty(SOCKET client)
static void do_upgrade(SOCKET client)
{
static const char *filename = "testagentd.tmp";
- static const char* upgrade_script = "./replace.bat";
int fd, success;
if (!expect_list_size(client, 1)
@@ -1103,34 +1102,26 @@ static void do_upgrade(SOCKET client)
close(fd);
}
- if (!success)
- unlink(filename);
- else
- success = platform_upgrade_script(upgrade_script, filename, server_argv);
+ if (success)
+ success = platform_upgrade(filename, server_argv);
if (success)
{
- char* args[2];
- char* redirects[3] = {"", "", ""};
-
send_list_size(client, 0);
- args[0] = strdup(upgrade_script);
- args[1] = NULL;
- success = platform_run(args, RUN_DNT, redirects);
- free(args[0]);
- if (success)
- {
- /* Decrement the start count since this one is intentional */
- start_count--;
- save_start_count();
+ /* Decrement the start count since this one is intentional */
+ start_count--;
+ save_start_count();
- broken = 1;
- quit = 1;
- }
+ /* Tell the main loop to quit */
+ broken = 1;
+ quit = 1;
}
else
+ {
send_error(client);
+ unlink(filename);
+ }
}
static void do_getcwd(SOCKET client)
More information about the wine-cvs
mailing list