tools/dbus-run-session: fix race between manual and automatically started dbus-daemon...

dbus-run-session starts a dbus-daemon before the client application.
We must avoid letting the application try to connect before the
dbus-daemon's DBusServer is listening for connections.

In the Unix implementation, we already achieved this via the
--print-address option. If the client tried to connect too soon,
the server would not yet be listening and the client would fail.

In the Windows implementation, we communicate the bus address to
the client application as an autolaunch: address, so if the client
tried to connect too soon, it would autolaunch a new dbus-daemon
instead of using the one that it was intended to use.

We can avoid this by using a new option to pass in a Windows event
object, which will be set when the server has started and is ready
to process connections.

Fixes #297

diff --git a/bus/bus.c b/bus/bus.c
index db20bbbc2760c58ebf150ffba8bdfe6b42db5572..ea508d726ec447454f4a081d41af08d780fbec54 100644 (file)
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -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)
     {

diff --git a/bus/bus.h b/bus/bus.h
index 99625ca310194015e92c39f4f93eb86fd07174a8..b44111809f4d3d13c3aa45c2021bd4be62e975f7 100644 (file)
--- a/bus/bus.h
+++ b/bus/bus.h
@@ -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,

diff --git a/bus/main.c b/bus/main.c
index 84f601b3086fbaba543d1065c0454d26672f8102..5f756d5c92c58e48b5eac02c477c8ad5d0d15807 100644 (file)
--- a/bus/main.c
+++ b/bus/main.c
@@ -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);

diff --git a/bus/test.c b/bus/test.c
index 42651e77407a358a88a4b2ca7bb7906f363b1aac..8d413fb5804bbab1c2d95a4b22dc1b478ebd6347 100644 (file)
--- a/bus/test.c
+++ b/bus/test.c
@@ -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 ca79605592fca910739676f0e7409ee7ee9e31d2..07d03bace564a1aa25452ec200b72d03035fbaa4 100644 (file)
--- 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-win.h b/dbus/dbus-sysdeps-win.h
index e9932ff5c8463e5a509e2398faecde9582c0b771..cbacd60cbf08fcd167fc4a92ee5631e12b277028 100644 (file)
--- a/dbus/dbus-sysdeps-win.h
+++ b/dbus/dbus-sysdeps-win.h
@@ -93,7 +93,10 @@ 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,

diff --git a/doc/dbus-daemon.1.xml.in b/doc/dbus-daemon.1.xml.in
index a9c0b5d5e820ea23aabf35d1805a1bc81d26447b..80fe945312e3f35caf6a286c0f0d1330998ff473 100644 (file)
--- 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 43d4df439e0b52994cc5acc96273046bdeff0b78..5caf5de5ffc70e77d1944be2ce933fb7cae2734d 100644 (file)
--- 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 1337ebcee85cd614093a51cef2571b25d5c8b32e..f8660c06b1da7f8bccccb696e54f7e621757e14f 100644 (file)
--- 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 d401e23eb64053c451b9ccb37f5185b4f95b0a71..c8b2920e6a5b9f877c3c923fa1fb952107b23ab9 100644 (file)
--- 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,7 +498,7 @@ 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 ());
@@ -490,14 +514,20 @@ run_session (const char *dbus_daemon,
   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 32020324af705fa68fb1e68be886fa3e6b80f73e..4fcfcb9fe446fb3d54da648778db46482bc8e779 100644 (file)
--- 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 e6397ffb0a8377e7efa804410250d6ecc20fe88a..12a3fd6c25bea0cae479335620ac544c3624e4bd 100644 (file)
--- 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