From eaf231b7db86954f38f8d2292f5586f12978fdf6 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Fri, 29 Apr 2016 09:47:51 +0200 Subject: [PATCH 1/7] Fix assert in test-spawn caused by missing initialization of DBusError instance on gcc builds. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95160 Reviewed-by: Simon McVittie (cherry picked from commit 9323a621e868d6a5b628b89696b1efe0300ff939) --- test/spawn-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spawn-test.c b/test/spawn-test.c index 723a4889..12d37c80 100644 --- a/test/spawn-test.c +++ b/test/spawn-test.c @@ -16,7 +16,7 @@ main (int argc, char **argv) { char **argv_copy; int i; - DBusError error; + DBusError error = DBUS_ERROR_INIT; if (argc < 2) { From cdf9fd02d22e81b92046f42d52db01b5a1bd79c7 Mon Sep 17 00:00:00 2001 From: Yiyang Fei Date: Wed, 27 Apr 2016 08:10:06 -0700 Subject: [PATCH 2/7] Fix crash in test-spawn unit test app on Windows. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95160 Reviewed-by: Simon McVittie --- dbus/dbus-spawn-win.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c index c58bf3cd..804aa426 100644 --- a/dbus/dbus-spawn-win.c +++ b/dbus/dbus-spawn-win.c @@ -659,7 +659,8 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, _DBUS_ASSERT_ERROR_IS_CLEAR (error); _dbus_assert (argv[0] != NULL); - *sitter_p = NULL; + if (sitter_p != NULL) + *sitter_p = NULL; PING(); sitter = _dbus_babysitter_new (); From 5ef167c1399499cf12d5800855cbe80f5b2439b6 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Mon, 9 May 2016 17:31:17 +0200 Subject: [PATCH 3/7] Suppress Windows popups and jit debugger when app crashes with exception. Based on a patch from Yiyang Fei. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95155 Reviewed-by: Simon McVittie --- test/test-segfault.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/test-segfault.c b/test/test-segfault.c index c062ce1c..7e2e82a1 100644 --- a/test/test-segfault.c +++ b/test/test-segfault.c @@ -13,11 +13,36 @@ #include #endif +#ifdef DBUS_WIN +#include + +int +exception_handler(LPEXCEPTION_POINTERS p); + +/* Explicit Windows exception handlers needed to supress OS popups */ +int +exception_handler(LPEXCEPTION_POINTERS p) +{ + fprintf(stderr, "test-segfault: raised fatal exception as intended\n"); + ExitProcess(0xc0000005); +} +#endif + int main (int argc, char **argv) { char *p; +#ifdef DBUS_WIN + /* Disable Windows popup dialog when an app crashes so that app quits + * immediately with error code instead of waiting for user to dismiss + * the dialog. */ + DWORD dwMode = SetErrorMode(SEM_NOGPFAULTERRORBOX); + SetErrorMode(dwMode | SEM_NOGPFAULTERRORBOX); + /* Disable "just in time" debugger */ + SetUnhandledExceptionFilter((LPTOP_LEVEL_EXCEPTION_FILTER)&exception_handler); +#endif + #if HAVE_SETRLIMIT /* No core dumps please, we know we crashed. */ struct rlimit r = { 0, }; From a5c51278add34ea57e194c778efb478502578e8f Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Fri, 13 May 2016 00:56:42 +0200 Subject: [PATCH 4/7] Eliminates a race condition accessing DBusBabysitter instance at startup of babysitter() on Windows. Ensure that the babysitter thread already owns its one reference to the babysitter when it starts up, and eliminates the race condition. This patch requires that DBusBabysitter refcounting is thread-safe and is based on an analysis and proposal of Simon Mc Vittie. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95191 Reviewed-by: Simon McVittie --- dbus/dbus-spawn-win.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c index 804aa426..fa290638 100644 --- a/dbus/dbus-spawn-win.c +++ b/dbus/dbus-spawn-win.c @@ -586,8 +586,6 @@ babysitter (void *parameter) DBusBabysitter *sitter = (DBusBabysitter *) parameter; PING(); - _dbus_babysitter_ref (sitter); - if (sitter->child_setup) { PING(); @@ -728,7 +726,7 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, PING(); sitter_thread = (HANDLE) CreateThread (NULL, 0, babysitter, - sitter, 0, &sitter_thread_id); + _dbus_babysitter_ref (sitter), 0, &sitter_thread_id); if (sitter_thread == 0) { From baeea825a4a84858f7df2755853e0b7d2d2c91e7 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Tue, 10 May 2016 16:53:57 +0200 Subject: [PATCH 5/7] On Windows make access to member 'refcount' of struct DBusBabysitter thread safe. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95191 Reviewed-by: Simon McVittie --- dbus/dbus-internals.h | 1 + dbus/dbus-spawn-win.c | 35 ++++++++++++++++++++++++++++------- dbus/dbus-sysdeps.h | 3 +++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index df8e9643..5e6efd81 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -124,6 +124,7 @@ static void _dbus_verbose(const char * x,...) {;} # define _dbus_is_verbose() FALSE #endif /* !DBUS_ENABLE_VERBOSE_MODE */ +DBUS_PRIVATE_EXPORT void _dbus_trace_ref (const char *obj_name, void *obj, int old_refcount, diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c index fa290638..cbd26f18 100644 --- a/dbus/dbus-spawn-win.c +++ b/dbus/dbus-spawn-win.c @@ -60,7 +60,7 @@ */ struct DBusBabysitter { - int refcount; + DBusAtomic refcount; HANDLE start_sync_event; #ifdef DBUS_ENABLE_EMBEDDED_TESTS @@ -91,16 +91,33 @@ struct DBusBabysitter int child_status; }; +static void +_dbus_babysitter_trace_ref (DBusBabysitter *sitter, + int old_refcount, + int new_refcount, + const char *why) +{ +#ifdef DBUS_ENABLE_VERBOSE_MODE + static int enabled = -1; + + _dbus_trace_ref ("DBusBabysitter", sitter, old_refcount, new_refcount, why, + "DBUS_BABYSITTER_TRACE", &enabled); +#endif +} + static DBusBabysitter* _dbus_babysitter_new (void) { DBusBabysitter *sitter; + dbus_int32_t old_refcount; sitter = dbus_new0 (DBusBabysitter, 1); if (sitter == NULL) return NULL; - sitter->refcount = 1; + old_refcount = _dbus_atomic_inc (&sitter->refcount); + + _dbus_babysitter_trace_ref (sitter, old_refcount, old_refcount+1, __FUNCTION__); sitter->start_sync_event = CreateEvent (NULL, FALSE, FALSE, NULL); if (sitter->start_sync_event == NULL) @@ -148,11 +165,13 @@ _dbus_babysitter_new (void) DBusBabysitter * _dbus_babysitter_ref (DBusBabysitter *sitter) { + dbus_int32_t old_refcount; PING(); _dbus_assert (sitter != NULL); - _dbus_assert (sitter->refcount > 0); - sitter->refcount += 1; + old_refcount = _dbus_atomic_inc (&sitter->refcount); + _dbus_assert (old_refcount > 0); + _dbus_babysitter_trace_ref (sitter, old_refcount, old_refcount+1, __FUNCTION__); return sitter; } @@ -187,14 +206,16 @@ void _dbus_babysitter_unref (DBusBabysitter *sitter) { int i; + dbus_int32_t old_refcount; PING(); _dbus_assert (sitter != NULL); - _dbus_assert (sitter->refcount > 0); - sitter->refcount -= 1; + old_refcount = _dbus_atomic_dec (&sitter->refcount); + _dbus_assert (old_refcount > 0); + _dbus_babysitter_trace_ref (sitter, old_refcount, old_refcount-1, __FUNCTION__); - if (sitter->refcount == 0) + if (old_refcount == 1) { close_socket_to_babysitter (sitter); diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 2f493b09..2d152b8c 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -295,8 +295,11 @@ struct DBusAtomic # undef DBUS_HAVE_ATOMIC_INT #endif +DBUS_PRIVATE_EXPORT dbus_int32_t _dbus_atomic_inc (DBusAtomic *atomic); +DBUS_PRIVATE_EXPORT dbus_int32_t _dbus_atomic_dec (DBusAtomic *atomic); +DBUS_PRIVATE_EXPORT dbus_int32_t _dbus_atomic_get (DBusAtomic *atomic); #ifdef DBUS_WIN From 7bcaf35bcaf8d594950df9b4a92b6f822ebfcabc Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Mon, 16 May 2016 12:52:25 +0200 Subject: [PATCH 6/7] Fix ambiguous setup of DBusBabySitter struct member child_handle on Windows. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95191 Reviewed-by: Simon McVittie --- dbus/dbus-spawn-win.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dbus/dbus-spawn-win.c b/dbus/dbus-spawn-win.c index cbd26f18..f741f923 100644 --- a/dbus/dbus-spawn-win.c +++ b/dbus/dbus-spawn-win.c @@ -605,6 +605,7 @@ babysitter (void *parameter) { int ret = 0; DBusBabysitter *sitter = (DBusBabysitter *) parameter; + HANDLE handle; PING(); if (sitter->child_setup) @@ -616,11 +617,14 @@ babysitter (void *parameter) _dbus_verbose ("babysitter: spawning %s\n", sitter->log_name); PING(); - sitter->child_handle = spawn_program (sitter->log_name, - sitter->argv, sitter->envp); + handle = spawn_program (sitter->log_name, sitter->argv, sitter->envp); PING(); - if (sitter->child_handle == (HANDLE) -1) + if (handle != INVALID_HANDLE_VALUE) + { + sitter->child_handle = handle; + } + else { sitter->child_handle = NULL; sitter->have_spawn_errno = TRUE; From 78362cfc62ebc5af8b529048b2e2aec0f977ceeb Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 16 May 2016 14:10:36 +0100 Subject: [PATCH 7/7] test-segfault: add missing include of on Windows Needed for fprintf (stderr, ...). Signed-off-by: Simon McVittie Reviewed-by: Ralf Habacker (cherry picked from commit 07b7dcd7178f927cd0b3a3282396b7f99c0b1d29) --- test/test-segfault.c | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test-segfault.c b/test/test-segfault.c index 7e2e82a1..b6c0f8ad 100644 --- a/test/test-segfault.c +++ b/test/test-segfault.c @@ -14,6 +14,7 @@ #endif #ifdef DBUS_WIN +#include #include int