diff options
author | Simon McVittie <smcv@collabora.com> | 2021-11-23 14:04:22 +0000 |
---|---|---|
committer | Simon McVittie <smcv@collabora.com> | 2021-11-23 14:04:22 +0000 |
commit | d4203c3ee59ebfbb20dab2f1abd9e517eccae4fd (patch) | |
tree | 3b83655f13c090963a3627f607231c84201ad9d0 | |
parent | 18b8883213179a5a14272b2c9dc7772b32e8a549 (diff) | |
parent | 2334307fbd093ad75f23cff19d9e44a956cccf68 (diff) |
Merge branch 'dbus-run-session-add-delay' into 'master'
tools/dbus-run-session: fix race between manual and automatically started dbus-daemon on Windows
Closes #297
See merge request dbus/dbus!195
-rw-r--r-- | bus/bus.c | 13 | ||||
-rw-r--r-- | bus/bus.h | 1 | ||||
-rw-r--r-- | bus/main.c | 26 | ||||
-rw-r--r-- | bus/test.c | 2 | ||||
-rw-r--r-- | dbus/dbus-spawn-win.c | 12 | ||||
-rw-r--r-- | dbus/dbus-sysdeps-util-win.c | 13 | ||||
-rw-r--r-- | dbus/dbus-sysdeps-win.c | 171 | ||||
-rw-r--r-- | dbus/dbus-sysdeps-win.h | 22 | ||||
-rw-r--r-- | doc/dbus-daemon.1.xml.in | 24 | ||||
-rw-r--r-- | tools/CMakeLists.txt | 2 | ||||
-rw-r--r-- | tools/Makefile.am | 5 | ||||
-rw-r--r-- | tools/dbus-run-session.c | 48 | ||||
-rw-r--r-- | tools/tool-common.c | 7 | ||||
-rw-r--r-- | tools/tool-common.h | 1 |
14 files changed, 314 insertions, 33 deletions
@@ -764,11 +764,13 @@ process_config_postinit (BusContext *context, return TRUE; } +/* Takes ownership of print_addr_pipe fds, print_pid_pipe fds and ready_event_handle */ BusContext* bus_context_new (const DBusString *config_file, BusContextFlags flags, DBusPipe *print_addr_pipe, DBusPipe *print_pid_pipe, + void *ready_event_handle, const DBusString *address, DBusError *error) { @@ -891,6 +893,17 @@ bus_context_new (const DBusString *config_file, _dbus_string_free (&addr); } +#ifdef DBUS_WIN + if (ready_event_handle != NULL) + { + _dbus_verbose ("Notifying that we are ready to receive connections (event handle=%p)\n", ready_event_handle); + if (!_dbus_win_event_set (ready_event_handle, error)) + goto failed; + if (!_dbus_win_event_free (ready_event_handle, error)) + goto failed; + } +#endif + context->connections = bus_connections_new (context); if (context->connections == NULL) { @@ -88,6 +88,7 @@ BusContext* bus_context_new (const DBusStri BusContextFlags flags, DBusPipe *print_addr_pipe, DBusPipe *print_pid_pipe, + void *ready_event_handle, const DBusString *address, DBusError *error); dbus_bool_t bus_context_reload_config (BusContext *context, @@ -48,6 +48,10 @@ static BusContext *context; +#ifdef DBUS_WIN +#include <dbus/dbus-sysdeps-win.h> +#endif + #ifdef DBUS_UNIX /* Despite its name and its unidirectional nature, this is actually @@ -163,6 +167,9 @@ usage (void) " [--syslog]" " [--syslog-only]" " [--nofork]" +#ifdef DBUS_WIN + " [--ready-event-handle=value]" +#endif #ifdef DBUS_UNIX " [--fork]" " [--systemd-activation]" @@ -403,6 +410,8 @@ main (int argc, char **argv) dbus_bool_t print_address; dbus_bool_t print_pid; BusContextFlags flags; + void *ready_event_handle; + #ifdef DBUS_UNIX const char *error_str; @@ -428,6 +437,7 @@ main (int argc, char **argv) * to inherit fds we might have inherited from our caller. */ _dbus_fd_set_all_close_on_exec (); #endif + ready_event_handle = NULL; if (!_dbus_string_init (&config_file)) return 1; @@ -619,6 +629,20 @@ main (int argc, char **argv) { print_pid = TRUE; /* and we'll get the next arg if appropriate */ } +#ifdef DBUS_WIN + else if (strstr (arg, "--ready-event-handle=") == arg) + { + const char *desc; + desc = strchr (arg, '='); + _dbus_assert (desc != NULL); + ++desc; + if (sscanf (desc, "%p", &ready_event_handle) != 1) + { + fprintf (stderr, "%s specified, but invalid handle provided\n", arg); + exit (1); + } + } +#endif else { usage (); @@ -693,7 +717,7 @@ main (int argc, char **argv) dbus_error_init (&error); context = bus_context_new (&config_file, flags, - &print_addr_pipe, &print_pid_pipe, + &print_addr_pipe, &print_pid_pipe, ready_event_handle, _dbus_string_get_length(&address) > 0 ? &address : NULL, &error); _dbus_string_free (&config_file); @@ -296,7 +296,7 @@ bus_context_new_test (const DBusString *test_data_dir, } dbus_error_init (&error); - context = bus_context_new (&config_file, BUS_CONTEXT_FLAG_NONE, NULL, NULL, NULL, &error); + context = bus_context_new (&config_file, BUS_CONTEXT_FLAG_NONE, NULL, NULL, NULL, NULL, &error); if (context == NULL) { _DBUS_ASSERT_ERROR_IS_SET (&error); diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c index ca796055..07d03bac 100644 --- a/dbus/dbus-spawn-win.c +++ b/dbus/dbus-spawn-win.c @@ -497,7 +497,8 @@ build_env_string (char** envp) HANDLE _dbus_spawn_program (const char *name, char **argv, - char **envp) + char **envp, + dbus_bool_t inherit_handles) { PROCESS_INFORMATION pi = { NULL, 0, 0, 0 }; STARTUPINFOA si; @@ -531,7 +532,12 @@ _dbus_spawn_program (const char *name, #ifdef DBUS_WINCE result = CreateProcessA (name, arg_string, NULL, NULL, FALSE, 0, #else - result = CreateProcessA (NULL, arg_string, NULL, NULL, FALSE, 0, + result = CreateProcessA (NULL, /* no application name */ + arg_string, + NULL, /* no process attributes */ + NULL, /* no thread attributes */ + inherit_handles, /* inherit handles */ + 0, /* flags */ #endif (LPVOID)env_string, NULL, &si, &pi); free (arg_string); @@ -666,7 +672,7 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, _dbus_verbose ("babysitter: spawn child '%s'\n", my_argv[0]); PING(); - handle = _dbus_spawn_program (sitter->log_name, my_argv, (char **) envp); + handle = _dbus_spawn_program (sitter->log_name, my_argv, (char **) envp, FALSE); if (my_argv != NULL) { diff --git a/dbus/dbus-sysdeps-util-win.c b/dbus/dbus-sysdeps-util-win.c index 6bb62aed..4e23b494 100644 --- a/dbus/dbus-sysdeps-util-win.c +++ b/dbus/dbus-sysdeps-util-win.c @@ -1674,16 +1674,3 @@ void _dbus_daemon_report_stopping (void) { } - -void -_dbus_win_stderr_win_error (const char *app, - const char *message, - unsigned long code) -{ - DBusError error; - - dbus_error_init (&error); - _dbus_win_set_error_from_win_error (&error, code); - fprintf (stderr, "%s: %s: %s\n", app, message, error.message); - dbus_error_free (&error); -} diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c index 10a3c4a8..650af7fc 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -6,7 +6,7 @@ * Copyright (C) 2005 Novell, Inc. * Copyright (C) 2006 Peter Kümmel <syntheticpp@gmx.net> * Copyright (C) 2006 Christian Ehrlicher <ch.ehrlicher@gmx.de> - * Copyright (C) 2006-2013 Ralf Habacker <ralf.habacker@freenet.de> + * Copyright (C) 2006-2021 Ralf Habacker <ralf.habacker@freenet.de> * * Licensed under the Academic Free License version 2.1 * @@ -4009,5 +4009,174 @@ _dbus_get_low_level_socket_errno (void) return WSAGetLastError (); } +void +_dbus_win_set_error_from_last_error (DBusError *error, + const char *format, + ...) +{ + const char *name; + char *message = NULL; + + if (error == NULL) + return; + + /* make sure to do this first, in case subsequent library calls overwrite GetLastError() */ + name = _dbus_win_error_from_last_error (); + message = _dbus_win_error_string (GetLastError ()); + + if (format != NULL) + { + DBusString str; + va_list args; + dbus_bool_t retval; + + if (!_dbus_string_init (&str)) + goto out; + + va_start (args, format); + retval = _dbus_string_append_printf_valist (&str, format, args); + va_end (args); + if (!retval) + { + _dbus_string_free (&str); + goto out; + } + + dbus_set_error (error, name, "%s: %s", _dbus_string_get_const_data (&str), message); + _dbus_string_free (&str); + } + else + { + dbus_set_error (error, name, "%s", message); + } + +out: + if (message != NULL) + _dbus_win_free_error_string (message); +} + +/** + * Creates a Windows event object and returns the corresponding handle + * + * The returned object is unnamed, is a manual-reset event object, + * is initially in the non-signalled state, and is inheritable by child + * processes. + * + * @param error the error to set + * @return handle for the created event + * @return #NULL if an error has occurred, the reason is returned in \p error + */ +HANDLE +_dbus_win_event_create_inheritable (DBusError *error) +{ + HANDLE handle; + + handle = CreateEvent (NULL, TRUE, FALSE, NULL); + if (handle == NULL) + { + _dbus_win_set_error_from_last_error (error, "Could not create event"); + return NULL; + } + else if (GetLastError () == ERROR_ALREADY_EXISTS) + { + _dbus_win_set_error_from_last_error (error, "Event already exists"); + return NULL; + } + + if (!SetHandleInformation (handle, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)) + { + _dbus_win_set_error_from_last_error (error, "Could not set inheritance for event %s", handle); + CloseHandle (handle); + return NULL; + } + return handle; +} + +/** + * Set a Windows event to the signalled state + * + * @param handle the handle for the event to be set + * @return TRUE the event was set successfully + * @return FALSE an error has occurred, the reason is returned in \p error + */ +dbus_bool_t +_dbus_win_event_set (HANDLE handle, DBusError *error) +{ + _dbus_assert (handle != NULL); + + if (!SetEvent (handle)) + { + _dbus_win_set_error_from_last_error (error, "Could not trigger event (handle %p)", handle); + return FALSE; + } + return TRUE; +} + +/** + * Wait for a Windows event to enter the signalled state + * + * @param handle the handle for the event to wait for + * @param timeout the waiting time in milliseconds, or INFINITE to wait forever, + * or 0 to check immediately and not wait (polling) + * @param error the error to set + * @return TRUE the event was set successfully + * @return FALSE an error has occurred, the reason is returned in \p error + */ +dbus_bool_t +_dbus_win_event_wait (HANDLE handle, int timeout, DBusError *error) +{ + DWORD status; + + _dbus_assert (handle != NULL); + + status = WaitForSingleObject (handle, timeout); + switch (status) + { + case WAIT_OBJECT_0: + return TRUE; + + case WAIT_FAILED: + { + _dbus_win_set_error_from_last_error (error, "Unable to wait for event (handle %p)", handle); + return FALSE; + } + + case WAIT_TIMEOUT: + /* GetLastError() is not set */ + dbus_set_error (error, DBUS_ERROR_TIMEOUT, "Timed out waiting for event (handle %p)", handle); + return FALSE; + + default: + /* GetLastError() is probably not set? */ + dbus_set_error (error, DBUS_ERROR_FAILED, "Unknown result '%lu' while waiting for event (handle %p)", status, handle); + return FALSE; + } +} + +/** + * Delete a Windows event + * + * @param handle handle for the event to delete + * @param error the error to set (optional) + * @return TRUE the event has been deleted successfully or the handle specifies a #NULL or invalid handle + * @return FALSE an error has occurred, the reason is returned in \p error if specified + */ +dbus_bool_t +_dbus_win_event_free (HANDLE handle, DBusError *error) +{ + if (handle == NULL || handle == INVALID_HANDLE_VALUE) + return TRUE; + + if (CloseHandle (handle)) + return TRUE; + + /* the handle may already be closed */ + if (GetLastError () == ERROR_INVALID_HANDLE) + return TRUE; + + _dbus_win_set_error_from_last_error (error, "Could not close event (handle %p)", handle); + return FALSE; +} + /** @} end of sysdeps-win */ /* tests in dbus-sysdeps-util.c */ diff --git a/dbus/dbus-sysdeps-win.h b/dbus/dbus-sysdeps-win.h index edce99a3..2775cfc6 100644 --- a/dbus/dbus-sysdeps-win.h +++ b/dbus/dbus-sysdeps-win.h @@ -46,9 +46,6 @@ const char* _dbus_win_error_from_last_error (void); dbus_bool_t _dbus_win_startup_winsock (void); void _dbus_win_warn_win_error (const char *message, unsigned long code); -void _dbus_win_stderr_win_error (const char *app, - const char *message, - unsigned long code); DBUS_PRIVATE_EXPORT char * _dbus_win_error_string (int error_number); DBUS_PRIVATE_EXPORT @@ -93,7 +90,24 @@ void _dbus_threads_windows_ensure_ctor_linked (void); DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_getsid(char **sid, dbus_pid_t process_id); -HANDLE _dbus_spawn_program (const char *name, char **argv, char **envp); +HANDLE _dbus_spawn_program (const char *name, + char **argv, + char **envp, + dbus_bool_t inherit_handles); + +DBUS_PRIVATE_EXPORT +void _dbus_win_set_error_from_last_error (DBusError *error, + const char *format, + ...); + +DBUS_PRIVATE_EXPORT +HANDLE _dbus_win_event_create_inheritable (DBusError *error); +DBUS_PRIVATE_EXPORT +dbus_bool_t _dbus_win_event_set (HANDLE handle, DBusError *error); +DBUS_PRIVATE_EXPORT +dbus_bool_t _dbus_win_event_wait (HANDLE handle, int timeout, DBusError *error); +DBUS_PRIVATE_EXPORT +dbus_bool_t _dbus_win_event_free (HANDLE handle, DBusError *error); #endif diff --git a/doc/dbus-daemon.1.xml.in b/doc/dbus-daemon.1.xml.in index a9c0b5d5..80fe9453 100644 --- a/doc/dbus-daemon.1.xml.in +++ b/doc/dbus-daemon.1.xml.in @@ -32,6 +32,7 @@ <arg choice='opt'>--nosyslog </arg> <arg choice='opt'>--syslog </arg> <arg choice='opt'>--syslog-only </arg> + <arg choice='opt'>--ready-event-handle=value</arg> <sbr/> </cmdsynopsis> </refsynopsisdiv> @@ -199,6 +200,29 @@ files.</para> </listitem> </varlistentry> + <varlistentry> + <term><option>--ready-event-handle=value</option></term> + <listitem> + <para>With this option, the dbus daemon raises an event when it is ready to process + connections. The <replaceable>handle</replaceable> must be the Windows handle + for an event object, in the format printed by the <function>printf</function> + format string <literal>%p</literal>. The parent process must create this event + object (for example with the <function>CreateEvent</function> function) in a + nonsignaled state, then configure it to be inherited by the dbus-daemon process. + The dbus-daemon will signal the event as if via <function>SetEvent</function> + when it is ready to receive connections from clients. The parent process can + wait for this to occur by using functions such as + <function>WaitForSingleObject</function>. + This option is only supported under Windows. On Unix platforms, + a similar result can be achieved by waiting for the address and/or + process ID to be printed to the inherited file descriptors used + for <option>--print-address</option> and/or <option>--print-pid</option>. + </para> + </listitem> + </varlistentry> + + + </variablelist> </refsect1> diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index 43d4df43..5caf5de5 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -57,6 +57,8 @@ set(dbus_cleanup_sockets_SOURCES set(dbus_run_session_SOURCES dbus-run-session.c + tool-common.c + tool-common.h ) set(dbus_uuidgen_SOURCES diff --git a/tools/Makefile.am b/tools/Makefile.am index 1337ebce..f8660c06 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -71,7 +71,10 @@ dbus_launch_LDADD = \ $(NULL) dbus_run_session_SOURCES = \ - dbus-run-session.c + dbus-run-session.c \ + tool-common.c \ + tool-common.h + $(NULL) dbus_run_session_LDADD = \ $(CODE_COVERAGE_LIBS) \ diff --git a/tools/dbus-run-session.c b/tools/dbus-run-session.c index d401e23e..46fa5c45 100644 --- a/tools/dbus-run-session.c +++ b/tools/dbus-run-session.c @@ -4,7 +4,7 @@ * Copyright © 2003-2006 Red Hat, Inc. * Copyright © 2006 Thiago Macieira <thiago@kde.org> * Copyright © 2011-2012 Nokia Corporation - * Copyright © 2018 Ralf Habacker + * Copyright © 2018, 2021 Ralf Habacker * * Licensed under the Academic Free License version 2.1 * @@ -47,6 +47,8 @@ #include "dbus/dbus.h" #include "dbus/dbus-internals.h" +#include "tool-common.h" + #define MAX_ADDR_LEN 512 #define PIPE_READ_END 0 #define PIPE_WRITE_END 1 @@ -101,7 +103,7 @@ version (void) "Copyright (C) 2003-2006 Red Hat, Inc.\n" "Copyright (C) 2006 Thiago Macieira\n" "Copyright © 2011-2012 Nokia Corporation\n" - "Copyright © 2018 Ralf Habacker\n" + "Copyright © 2018, 2021 Ralf Habacker\n" "\n" "This is free software; see the source for copying conditions.\n" "There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n", @@ -381,10 +383,11 @@ run_session (const char *dbus_daemon, char **argv, int prog_arg) { - char *dbus_daemon_argv[4]; + char *dbus_daemon_argv[5]; int ret = 127; HANDLE server_handle = NULL; HANDLE app_handle = NULL; + HANDLE ready_event_handle = NULL; DWORD exit_code; DBusString argv_strings[4]; DBusString address; @@ -394,6 +397,7 @@ run_session (const char *dbus_daemon, dbus_bool_t result = TRUE; char *key = NULL; char *value = NULL; + DBusError error; if (!_dbus_string_init (&argv_strings[0])) result = FALSE; @@ -401,11 +405,20 @@ run_session (const char *dbus_daemon, result = FALSE; if (!_dbus_string_init (&argv_strings[2])) result = FALSE; + if (!_dbus_string_init (&argv_strings[3])) + result = FALSE; if (!_dbus_string_init (&address)) result = FALSE; if (!result) goto out; + /* The handle of this event is used by the dbus daemon + * to signal that connections are ready. */ + dbus_error_init (&error); + ready_event_handle = _dbus_win_event_create_inheritable (&error); + if (ready_event_handle == NULL) + goto out; + /* run dbus daemon */ _dbus_get_real_time (&sec, &usec); /* On Windows it's difficult to make use of --print-address to @@ -421,18 +434,29 @@ run_session (const char *dbus_daemon, else _dbus_string_append_printf (&argv_strings[1], "--session"); _dbus_string_append_printf (&argv_strings[2], "--address=%s", _dbus_string_get_const_data (&address)); + _dbus_string_append_printf (&argv_strings[3], "--ready-event-handle=%p", ready_event_handle); dbus_daemon_argv[0] = _dbus_string_get_data (&argv_strings[0]); dbus_daemon_argv[1] = _dbus_string_get_data (&argv_strings[1]); dbus_daemon_argv[2] = _dbus_string_get_data (&argv_strings[2]); - dbus_daemon_argv[3] = NULL; + dbus_daemon_argv[3] = _dbus_string_get_data (&argv_strings[3]); + dbus_daemon_argv[4] = NULL; - server_handle = _dbus_spawn_program (dbus_daemon, dbus_daemon_argv, NULL); + server_handle = _dbus_spawn_program (dbus_daemon, dbus_daemon_argv, NULL, TRUE); if (!server_handle) { - _dbus_win_stderr_win_error (me, "Could not start dbus daemon", GetLastError ()); + _dbus_win_set_error_from_last_error (&error, "Could not start dbus daemon"); goto out; } + /* wait until dbus-daemon is ready for connections */ + if (ready_event_handle != NULL) + { + _dbus_verbose ("Wait until dbus-daemon is ready for connections (event handle %p)\n", ready_event_handle); + if (!_dbus_win_event_wait (ready_event_handle, 30000, &error)) + goto out; + _dbus_verbose ("Got signal that dbus-daemon is ready for connections\n"); + } + /* run app */ env = _dbus_get_environment (); env_table = _dbus_hash_table_new (DBUS_HASH_STRING, @@ -474,30 +498,36 @@ run_session (const char *dbus_daemon, if (!env) goto out; - app_handle = _dbus_spawn_program (argv[prog_arg], argv + prog_arg, env); + app_handle = _dbus_spawn_program (argv[prog_arg], argv + prog_arg, env, FALSE); if (!app_handle) { - _dbus_win_stderr_win_error (me, "unable to start child process", GetLastError ()); + _dbus_win_set_error_from_last_error (&error, "Unable to start child process"); goto out; } WaitForSingleObject (app_handle, INFINITE); if (!GetExitCodeProcess (app_handle, &exit_code)) { - _dbus_win_stderr_win_error (me, "could not fetch exit code", GetLastError ()); + _dbus_win_set_error_from_last_error (&error, "Could not fetch exit code"); goto out; } ret = exit_code; out: + if (dbus_error_is_set (&error)) + tool_stderr_error (me, &error); + dbus_error_free (&error); TerminateProcess (server_handle, 0); if (server_handle != NULL) CloseHandle (server_handle); if (app_handle != NULL) CloseHandle (app_handle); + if (ready_event_handle != NULL) + _dbus_win_event_free (ready_event_handle, NULL); _dbus_string_free (&argv_strings[0]); _dbus_string_free (&argv_strings[1]); _dbus_string_free (&argv_strings[2]); + _dbus_string_free (&argv_strings[3]); _dbus_string_free (&address); dbus_free_string_array (env); if (env_table != NULL) diff --git a/tools/tool-common.c b/tools/tool-common.c index 32020324..4fcfcb9f 100644 --- a/tools/tool-common.c +++ b/tools/tool-common.c @@ -80,3 +80,10 @@ tool_write_all (int fd, return TRUE; } + +void +tool_stderr_error (const char *context, + DBusError *error) +{ + fprintf (stderr, "%s: %s: %s\n", context, error->name, error->message); +} diff --git a/tools/tool-common.h b/tools/tool-common.h index e6397ffb..12a3fd6c 100644 --- a/tools/tool-common.h +++ b/tools/tool-common.h @@ -34,5 +34,6 @@ void tool_oom (const char *doing) _DBUS_GNUC_NORETURN; dbus_bool_t tool_write_all (int fd, const void *buf, size_t size); +void tool_stderr_error (const char *context, DBusError *error); #endif |