Commit 37241fc0 authored by Ralf Habacker's avatar Ralf Habacker

Fix memory leak issue in spawn code on Windows.

In _dbus_babysitter_block_for_child_exit () use spawning
thread handle to have a reliable way to detect spawning
thread termination.

See https://bugs.freedesktop.org/show_bug.cgi?id=95191#c33
for more informations.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95191Reviewed-by: default avatarSimon McVittie <simon.mcvittie@collabora.co.uk>
parent 4858faf1
...@@ -63,10 +63,6 @@ struct DBusBabysitter ...@@ -63,10 +63,6 @@ struct DBusBabysitter
DBusAtomic refcount; DBusAtomic refcount;
HANDLE start_sync_event; HANDLE start_sync_event;
#ifdef DBUS_ENABLE_EMBEDDED_TESTS
HANDLE end_sync_event;
#endif
char *log_name; char *log_name;
...@@ -74,6 +70,7 @@ struct DBusBabysitter ...@@ -74,6 +70,7 @@ struct DBusBabysitter
char **argv; char **argv;
char **envp; char **envp;
HANDLE thread_handle;
HANDLE child_handle; HANDLE child_handle;
DBusSocket socket_to_babysitter; /* Connection to the babysitter thread */ DBusSocket socket_to_babysitter; /* Connection to the babysitter thread */
DBusSocket socket_to_main; DBusSocket socket_to_main;
...@@ -124,15 +121,6 @@ _dbus_babysitter_new (void) ...@@ -124,15 +121,6 @@ _dbus_babysitter_new (void)
return NULL; return NULL;
} }
#ifdef DBUS_ENABLE_EMBEDDED_TESTS
sitter->end_sync_event = CreateEvent (NULL, FALSE, FALSE, NULL);
if (sitter->end_sync_event == NULL)
{
_dbus_babysitter_unref (sitter);
return NULL;
}
#endif
sitter->child_handle = NULL; sitter->child_handle = NULL;
sitter->socket_to_babysitter = sitter->socket_to_main = _dbus_socket_get_invalid (); sitter->socket_to_babysitter = sitter->socket_to_main = _dbus_socket_get_invalid ();
...@@ -269,13 +257,11 @@ _dbus_babysitter_unref (DBusBabysitter *sitter) ...@@ -269,13 +257,11 @@ _dbus_babysitter_unref (DBusBabysitter *sitter)
sitter->start_sync_event = NULL; sitter->start_sync_event = NULL;
} }
#ifdef DBUS_ENABLE_EMBEDDED_TESTS if (sitter->thread_handle)
if (sitter->end_sync_event != NULL)
{ {
CloseHandle (sitter->end_sync_event); CloseHandle (sitter->thread_handle);
sitter->end_sync_event = NULL; sitter->thread_handle = NULL;
} }
#endif
dbus_free (sitter->log_name); dbus_free (sitter->log_name);
...@@ -646,10 +632,6 @@ babysitter (void *parameter) ...@@ -646,10 +632,6 @@ babysitter (void *parameter)
sitter->child_handle = NULL; sitter->child_handle = NULL;
} }
#ifdef DBUS_ENABLE_EMBEDDED_TESTS
SetEvent (sitter->end_sync_event);
#endif
PING(); PING();
send (sitter->socket_to_main.sock, " ", 1, 0); send (sitter->socket_to_main.sock, " ", 1, 0);
...@@ -668,7 +650,6 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, ...@@ -668,7 +650,6 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p,
DBusError *error) DBusError *error)
{ {
DBusBabysitter *sitter; DBusBabysitter *sitter;
HANDLE sitter_thread;
DWORD sitter_thread_id; DWORD sitter_thread_id;
_DBUS_ASSERT_ERROR_IS_CLEAR (error); _DBUS_ASSERT_ERROR_IS_CLEAR (error);
...@@ -739,17 +720,16 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, ...@@ -739,17 +720,16 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p,
sitter->envp = envp; sitter->envp = envp;
PING(); PING();
sitter_thread = (HANDLE) CreateThread (NULL, 0, babysitter, sitter->thread_handle = (HANDLE) CreateThread (NULL, 0, babysitter,
_dbus_babysitter_ref (sitter), 0, &sitter_thread_id); _dbus_babysitter_ref (sitter), 0, &sitter_thread_id);
if (sitter_thread == 0) if (sitter->thread_handle == NULL)
{ {
PING(); PING();
dbus_set_error_const (error, DBUS_ERROR_SPAWN_FORK_FAILED, dbus_set_error_const (error, DBUS_ERROR_SPAWN_FORK_FAILED,
"Failed to create new thread"); "Failed to create new thread");
goto out0; goto out0;
} }
CloseHandle (sitter_thread);
PING(); PING();
WaitForSingleObject (sitter->start_sync_event, INFINITE); WaitForSingleObject (sitter->start_sync_event, INFINITE);
...@@ -811,10 +791,10 @@ get_test_exec (const char *exe, ...@@ -811,10 +791,10 @@ get_test_exec (const char *exe,
static void static void
_dbus_babysitter_block_for_child_exit (DBusBabysitter *sitter) _dbus_babysitter_block_for_child_exit (DBusBabysitter *sitter)
{ {
if (sitter->child_handle == NULL) /* The thread terminates after the child does. We want to wait for the thread,
return; * not just the child, to avoid data races and ensure that it has freed all
* its memory. */
WaitForSingleObject (sitter->end_sync_event, INFINITE); WaitForSingleObject (sitter->thread_handle, INFINITE);
} }
static dbus_bool_t static dbus_bool_t
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment