From 62097c3ea1261ad5ffefb8996b2c9bd09baeeab9 Mon Sep 17 00:00:00 2001 From: Keith Seitz Date: Wed, 21 Jun 2006 20:56:37 +0000 Subject: [PATCH 1/8] Implement thread suspend/resume API (Linux threads only) (Cherry-picked commit 42fe54a from 'gcc_boehmgc' branch.) * pthread_stop_world.c (GC_suspend_handler): Redirect to suspension routine if signal is received and thread is flagged SUSPENDED_EXT. * pthread_stop_world.c (GC_brief_async_signal_safe_sleep, suspend_self_inner, suspend_self, GC_suspend_thread, GC_resume_thread): New function. * include/gc.h (GC_suspend_thread, GC_resume_thread): Declare. * include/private/pthread_support.h (SUSPENDED_EXT): Update comment. Conflicts: * ChangeLog * include/gc.h * include/private/pthread_support.h * pthread_stop_world.c --- include/gc.h | 11 +++++ include/private/pthread_support.h | 4 +- pthread_stop_world.c | 73 +++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/include/gc.h b/include/gc.h index 7cf932034..d9f4e3974 100644 --- a/include/gc.h +++ b/include/gc.h @@ -1912,6 +1912,17 @@ GC_API void GC_CALL GC_win32_free_heap(void); (*GC_amiga_allocwrapper_do)(a,GC_malloc_atomic_ignore_off_page) #endif /* _AMIGA && !GC_AMIGA_MAKINGLIB */ +/* External thread suspension support. These functions do not implement + * suspension counts or any other higher-level abstraction. Threads which + * have been suspended numerous times will resume with the very first call + * to GC_resume_thread. + */ +#if defined(GC_PTHREADS) && !defined(__native_client__) \ + && !defined(GC_WIN32_THREADS) && !defined(GC_DARWIN_THREADS) +GC_API void GC_suspend_thread(pthread_t); +GC_API void GC_resume_thread(pthread_t); +#endif + #ifdef __cplusplus } /* end of extern "C" */ #endif diff --git a/include/private/pthread_support.h b/include/private/pthread_support.h index 294aeedb9..f835fa3c9 100644 --- a/include/private/pthread_support.h +++ b/include/private/pthread_support.h @@ -62,9 +62,7 @@ typedef struct GC_Thread_Rep { /* it unregisters itself, since it */ /* may not return a GC pointer. */ # define MAIN_THREAD 4 /* True for the original thread only. */ -# define SUSPENDED_EXT 8 /* Thread was suspended externally */ - /* (this is not used by the unmodified */ - /* GC itself at present). */ +# define SUSPENDED_EXT 8 /* Thread was suspended externally. */ # define DISABLED_GC 0x10 /* Collections are disabled while the */ /* thread is exiting. */ diff --git a/pthread_stop_world.c b/pthread_stop_world.c index 63c93c0f2..f9e86c55f 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -50,6 +50,8 @@ /* It's safe to call original pthread_sigmask() here. */ #undef pthread_sigmask +void suspend_self(); + #ifdef DEBUG_THREADS # ifndef NSIG # if defined(MAXSIG) @@ -210,6 +212,11 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy, void *context); #endif { int old_errno = errno; + GC_thread me = GC_lookup_thread (pthread_self()); + if (me -> flags & SUSPENDED_EXT) { + suspend_self(); + return; + } if (sig != GC_sig_suspend) { # if defined(GC_FREEBSD_THREADS) @@ -339,6 +346,72 @@ STATIC void GC_restart_handler(int sig) # endif } +#ifndef GC_TIME_LIMIT +# define GC_TIME_LIMIT 50 +#endif + +void GC_brief_async_signal_safe_sleep() +{ + struct timeval tv; + tv.tv_sec = 0; + tv.tv_usec = 1000 * GC_TIME_LIMIT / 2; + select(0, 0, 0, 0, &tv); +} + +static void *GC_CALLBACK suspend_self_inner(void *client_data) { + GC_thread me = (GC_thread)client_data; + + while (me -> flags & SUSPENDED_EXT) + GC_brief_async_signal_safe_sleep(); + return NULL; +} + +void suspend_self() { + GC_thread me = GC_lookup_thread(pthread_self()); + if (me == NULL) + ABORT("attempting to suspend unknown thread"); + + me -> flags |= SUSPENDED_EXT; + (void)GC_do_blocking(suspend_self_inner, me); +} + +#ifdef USE_TKILL_ON_ANDROID + static int android_thread_kill(pid_t tid, int sig); +#endif + +void GC_suspend_thread(pthread_t thread) { + if (thread == pthread_self()) + suspend_self(); + else { + int result; + GC_thread t = GC_lookup_thread(thread); + if (t == NULL) + ABORT("attempting to suspend unknown thread"); + + t -> flags |= SUSPENDED_EXT; +# ifndef USE_TKILL_ON_ANDROID + result = pthread_kill(t -> id, GC_sig_suspend); +# else + result = android_thread_kill(t -> kernel_id, GC_sig_suspend); +# endif + switch (result) { + case ESRCH: + case 0: + break; + default: + ABORT("pthread_kill failed"); + } + } +} + +void GC_resume_thread(pthread_t thread) { + GC_thread t = GC_lookup_thread(thread); + if (t == NULL) + ABORT("attempting to resume unknown thread"); + + t -> flags &= ~SUSPENDED_EXT; +} + #endif /* !GC_OPENBSD_UTHREADS && !NACL */ #ifdef IA64 From 0dae57c4db20c1ebc22d8cc6e9ed0531f6c12652 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Fri, 9 Oct 2015 21:59:40 +0300 Subject: [PATCH 2/8] Update AUTHORS file --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index fb3344fcd..10764a8c9 100644 --- a/AUTHORS +++ b/AUTHORS @@ -205,6 +205,7 @@ Kai Tietz Kaz Kojima Kazu Hirata Kazuhiro Inaoka +Keith Seitz Kenjiro Taura Kenneth Schalk Kevin Kenny From d34d5b53d67b3ef6fa2d70a945e21f023cb9e3e7 Mon Sep 17 00:00:00 2001 From: Keith Seitz Date: Mon, 23 Apr 2007 21:10:09 +0000 Subject: [PATCH 3/8] Add GC_is_thread_suspended API function (Cherry-picked commit 30c2f0e from 'gcc_boehmgc' branch.) * include/gc.h (GC_is_thread_suspended): Declare. * pthread_stop_world.c (GC_is_thread_suspended): New function. Conflicts: * ChangeLog * include/gc.h * pthread_stop_world.c --- include/gc.h | 1 + pthread_stop_world.c | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/include/gc.h b/include/gc.h index d9f4e3974..f8f764e12 100644 --- a/include/gc.h +++ b/include/gc.h @@ -1921,6 +1921,7 @@ GC_API void GC_CALL GC_win32_free_heap(void); && !defined(GC_WIN32_THREADS) && !defined(GC_DARWIN_THREADS) GC_API void GC_suspend_thread(pthread_t); GC_API void GC_resume_thread(pthread_t); +GC_API int GC_is_thread_suspended(pthread_t); #endif #ifdef __cplusplus diff --git a/pthread_stop_world.c b/pthread_stop_world.c index f9e86c55f..d80963320 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -412,6 +412,14 @@ void GC_resume_thread(pthread_t thread) { t -> flags &= ~SUSPENDED_EXT; } +int GC_is_thread_suspended(pthread_t thread) { + GC_thread t = GC_lookup_thread(thread); + if (t == NULL) + ABORT("querying suspension state of unknown thread"); + + return (t -> flags & SUSPENDED_EXT); +} + #endif /* !GC_OPENBSD_UTHREADS && !NACL */ #ifdef IA64 From 9f48082eafc4d54ca33390a72924715b03d7c1dd Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Thu, 29 Oct 2015 20:33:48 +0300 Subject: [PATCH 4/8] Code refactoring of thread suspend/resume API support * CMakeLists.txt (enable_gcj_support): Define GC_ENABLE_SUSPEND_THREAD. * configure.ac (enable_gcj_support): Likewise. * doc/README.macros (GC_ENABLE_SUSPEND_THREAD): Document. * include/gc.h (GC_suspend_thread, GC_resume_thread, GC_is_thread_suspended): Move to javaxfc.h. * include/gc_pthread_redirects.h (GC_SUSPEND_THREAD_ID): New macro. * include/javaxfc.h (GC_SUSPEND_THREAD_ID): Likewise. * include/javaxfc.h (GC_suspend_thread, GC_resume_thread, GC_is_thread_suspended): Define if and only if GC_THREADS; refine comment. * include/javaxfc.h (GC_suspend_thread, GC_resume_thread, GC_is_thread_suspended): Decorate with GC_CALL; change argument type from pthread_t to GC_SUSPEND_THREAD_ID. * pthread_stop_world.c (GC_suspend_thread, GC_resume_thread, GC_is_thread_suspended): Likewise. * pthread_stop_world.c (GC_suspend_handler): Move check for SUSPENDED_EXT to GC_suspend_handler_inner (to avoid duplicate GC_lookup_thread call). * pthread_stop_world.c (suspend_self_inner, GC_TIME_LIMIT, GC_brief_async_signal_safe_sleep, GC_suspend_thread, GC_resume_thread, GC_is_thread_suspended): Do not defined unless GC_ENABLE_SUSPEND_THREAD. * pthread_stop_world.c (suspend_self): Remove (invoke GC_do_blocking(suspend_self_inner) directly). * pthread_stop_world.c (GC_brief_async_signal_safe_sleep): Decorate with STATIC. * pthread_stop_world.c (GC_suspend_thread, GC_resume_thread, GC_is_thread_suspended): Wrap code into LOCK/UNLOCK (because, at least, GC_lookup_thread should be called with the allocation lock held). * pthread_stop_world.c (GC_suspend_thread, GC_resume_thread): Do not ABORT if thread is unregistered in GC (just no-op instead). * pthread_stop_world.c (GC_is_thread_suspended): Return 0 if thread is not registered in GC. --- CMakeLists.txt | 1 + configure.ac | 2 + doc/README.macros | 3 + include/gc.h | 12 --- include/gc_pthread_redirects.h | 4 + include/javaxfc.h | 15 ++++ pthread_stop_world.c | 157 ++++++++++++++++++--------------- 7 files changed, 112 insertions(+), 82 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bd7b92703..b7919c906 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -203,6 +203,7 @@ ENDIF(CMAKE_USE_WIN32_THREADS_INIT) OPTION(enable_gcj_support "Support for gcj" NO) IF(enable_gcj_support) ADD_DEFINITIONS("-DGC_GCJ_SUPPORT") + ADD_DEFINITIONS("-DGC_ENABLE_SUSPEND_THREAD") ENDIF(enable_gcj_support) diff --git a/configure.ac b/configure.ac index 3260e7dab..4ca7b0ab6 100644 --- a/configure.ac +++ b/configure.ac @@ -682,6 +682,8 @@ AC_ARG_ENABLE(gcj-support, [Disable support for gcj.])]) if test x"$enable_gcj_support" != xno; then AC_DEFINE(GC_GCJ_SUPPORT, 1, [Define to include support for gcj.]) + AC_DEFINE([GC_ENABLE_SUSPEND_THREAD], 1, + [Define to turn on GC_suspend_thread support.]) fi dnl Interaction with other programs that might use signals. diff --git a/doc/README.macros b/doc/README.macros index c3d27437c..90ef5128a 100644 --- a/doc/README.macros +++ b/doc/README.macros @@ -403,6 +403,9 @@ PARALLEL_MARK Allows the marker to run in multiple threads. Recommended GC_ALWAYS_MULTITHREADED Force multi-threaded mode at GC initialization. (Turns GC_allow_register_threads into a no-op routine.) +GC_ENABLE_SUSPEND_THREAD (Linux only) Turn on thread suspend/resume API +support. + GC_WINMAIN_REDIRECT (Win32 only) Redirect (rename) an application WinMain to GC_WinMain; implement the "real" WinMain which starts a new thread to call GC_WinMain after initializing the GC. Useful for WinCE. diff --git a/include/gc.h b/include/gc.h index f8f764e12..7cf932034 100644 --- a/include/gc.h +++ b/include/gc.h @@ -1912,18 +1912,6 @@ GC_API void GC_CALL GC_win32_free_heap(void); (*GC_amiga_allocwrapper_do)(a,GC_malloc_atomic_ignore_off_page) #endif /* _AMIGA && !GC_AMIGA_MAKINGLIB */ -/* External thread suspension support. These functions do not implement - * suspension counts or any other higher-level abstraction. Threads which - * have been suspended numerous times will resume with the very first call - * to GC_resume_thread. - */ -#if defined(GC_PTHREADS) && !defined(__native_client__) \ - && !defined(GC_WIN32_THREADS) && !defined(GC_DARWIN_THREADS) -GC_API void GC_suspend_thread(pthread_t); -GC_API void GC_resume_thread(pthread_t); -GC_API int GC_is_thread_suspended(pthread_t); -#endif - #ifdef __cplusplus } /* end of extern "C" */ #endif diff --git a/include/gc_pthread_redirects.h b/include/gc_pthread_redirects.h index 0d571e913..e069c578b 100644 --- a/include/gc_pthread_redirects.h +++ b/include/gc_pthread_redirects.h @@ -33,6 +33,10 @@ #ifndef GC_PTHREAD_REDIRECTS_ONLY # include +# ifndef GC_SUSPEND_THREAD_ID +# define GC_SUSPEND_THREAD_ID pthread_t +# endif + # ifndef GC_NO_DLOPEN # include GC_API void *GC_dlopen(const char * /* path */, int /* mode */); diff --git a/include/javaxfc.h b/include/javaxfc.h index 99eaf9ad8..9fd2f1884 100644 --- a/include/javaxfc.h +++ b/include/javaxfc.h @@ -40,6 +40,21 @@ */ GC_API void GC_CALL GC_finalize_all(void); +#ifdef GC_THREADS + /* External thread suspension support. No thread suspension count */ + /* (so a thread which has been suspended numerous times will be */ + /* resumed with the very first call to GC_resume_thread). */ + /* Acquire the allocation lock. Thread should be registered in GC */ + /* (otherwise no-op, GC_is_thread_suspended returns false). */ + /* Unimplemented on some platforms. Not recommended for general use. */ +# ifndef GC_SUSPEND_THREAD_ID +# define GC_SUSPEND_THREAD_ID void* +# endif + GC_API void GC_CALL GC_suspend_thread(GC_SUSPEND_THREAD_ID); + GC_API void GC_CALL GC_resume_thread(GC_SUSPEND_THREAD_ID); + GC_API int GC_CALL GC_is_thread_suspended(GC_SUSPEND_THREAD_ID); +#endif /* GC_THREADS */ + #ifdef __cplusplus } /* end of extern "C" */ #endif diff --git a/pthread_stop_world.c b/pthread_stop_world.c index d80963320..632c8ad4a 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -50,7 +50,9 @@ /* It's safe to call original pthread_sigmask() here. */ #undef pthread_sigmask -void suspend_self(); +#ifdef GC_ENABLE_SUSPEND_THREAD + static void *GC_CALLBACK suspend_self_inner(void *client_data); +#endif #ifdef DEBUG_THREADS # ifndef NSIG @@ -212,11 +214,6 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy, void *context); #endif { int old_errno = errno; - GC_thread me = GC_lookup_thread (pthread_self()); - if (me -> flags & SUSPENDED_EXT) { - suspend_self(); - return; - } if (sig != GC_sig_suspend) { # if defined(GC_FREEBSD_THREADS) @@ -267,6 +264,19 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED, /* of a thread which holds the allocation lock in order */ /* to stop the world. Thus concurrent modification of the */ /* data structure is impossible. */ + +# ifdef GC_ENABLE_SUSPEND_THREAD + if ((me -> flags & SUSPENDED_EXT) != 0) { + /* TODO: GC_with_callee_saves_pushed is redundant here. */ + (void)GC_do_blocking(suspend_self_inner, me); +# ifdef DEBUG_THREADS + GC_log_printf("Continuing %p on GC_resume_thread\n", (void *)self); +# endif + RESTORE_CANCEL(cancel_state); + return; + } +# endif + if (me -> stop_info.last_stop_count == my_stop_count) { /* Duplicate signal. OK if we are retrying. */ if (!GC_retry_signals) { @@ -346,79 +356,86 @@ STATIC void GC_restart_handler(int sig) # endif } -#ifndef GC_TIME_LIMIT -# define GC_TIME_LIMIT 50 -#endif - -void GC_brief_async_signal_safe_sleep() -{ - struct timeval tv; - tv.tv_sec = 0; - tv.tv_usec = 1000 * GC_TIME_LIMIT / 2; - select(0, 0, 0, 0, &tv); -} - -static void *GC_CALLBACK suspend_self_inner(void *client_data) { - GC_thread me = (GC_thread)client_data; - - while (me -> flags & SUSPENDED_EXT) - GC_brief_async_signal_safe_sleep(); - return NULL; -} +# ifdef GC_ENABLE_SUSPEND_THREAD +# ifndef GC_TIME_LIMIT +# define GC_TIME_LIMIT 50 +# endif -void suspend_self() { - GC_thread me = GC_lookup_thread(pthread_self()); - if (me == NULL) - ABORT("attempting to suspend unknown thread"); + STATIC void GC_brief_async_signal_safe_sleep(void) + { + struct timeval tv; + tv.tv_sec = 0; + tv.tv_usec = 1000 * GC_TIME_LIMIT / 2; + select(0, 0, 0, 0, &tv); + } - me -> flags |= SUSPENDED_EXT; - (void)GC_do_blocking(suspend_self_inner, me); -} + static void *GC_CALLBACK suspend_self_inner(void *client_data) { + GC_thread me = (GC_thread)client_data; -#ifdef USE_TKILL_ON_ANDROID - static int android_thread_kill(pid_t tid, int sig); -#endif - -void GC_suspend_thread(pthread_t thread) { - if (thread == pthread_self()) - suspend_self(); - else { - int result; - GC_thread t = GC_lookup_thread(thread); - if (t == NULL) - ABORT("attempting to suspend unknown thread"); + while ((me -> flags & SUSPENDED_EXT) != 0) { + /* TODO: Use sigsuspend() instead. */ + GC_brief_async_signal_safe_sleep(); + } + return NULL; + } - t -> flags |= SUSPENDED_EXT; -# ifndef USE_TKILL_ON_ANDROID - result = pthread_kill(t -> id, GC_sig_suspend); -# else - result = android_thread_kill(t -> kernel_id, GC_sig_suspend); +# ifdef USE_TKILL_ON_ANDROID + static int android_thread_kill(pid_t tid, int sig); # endif - switch (result) { - case ESRCH: - case 0: - break; - default: - ABORT("pthread_kill failed"); - } - } -} -void GC_resume_thread(pthread_t thread) { - GC_thread t = GC_lookup_thread(thread); - if (t == NULL) - ABORT("attempting to resume unknown thread"); + GC_API void GC_CALL GC_suspend_thread(GC_SUSPEND_THREAD_ID thread) { + GC_thread t; + int result; + DCL_LOCK_STATE; + + LOCK(); + t = GC_lookup_thread((pthread_t)thread); + if (t != NULL) { + t -> flags |= SUSPENDED_EXT; + if ((pthread_t)thread == pthread_self()) { + (void)GC_do_blocking(suspend_self_inner, t); + } else { +# ifndef USE_TKILL_ON_ANDROID + result = pthread_kill(t -> id, GC_sig_suspend); +# else + result = android_thread_kill(t -> kernel_id, GC_sig_suspend); +# endif + switch (result) { + case ESRCH: + case 0: + break; + default: + ABORT("pthread_kill failed"); + } + } + } + UNLOCK(); + } - t -> flags &= ~SUSPENDED_EXT; -} + GC_API void GC_CALL GC_resume_thread(GC_SUSPEND_THREAD_ID thread) { + GC_thread t; + DCL_LOCK_STATE; -int GC_is_thread_suspended(pthread_t thread) { - GC_thread t = GC_lookup_thread(thread); - if (t == NULL) - ABORT("querying suspension state of unknown thread"); + LOCK(); + t = GC_lookup_thread((pthread_t)thread); + if (t != NULL) + t -> flags &= ~SUSPENDED_EXT; + UNLOCK(); + } - return (t -> flags & SUSPENDED_EXT); -} + GC_API int GC_CALL GC_is_thread_suspended(GC_SUSPEND_THREAD_ID thread) { + GC_thread t; + int flags = 0; + DCL_LOCK_STATE; + + LOCK(); + t = GC_lookup_thread((pthread_t)thread); + if (t != NULL) + flags = t -> flags; + UNLOCK(); + return (flags & SUSPENDED_EXT) != 0; + } +# endif /* GC_ENABLE_SUSPEND_THREAD */ #endif /* !GC_OPENBSD_UTHREADS && !NACL */ From 8ff3262b23ff70e7ce53bccb90e20cfd4d39e171 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Mon, 2 Nov 2015 09:31:45 +0300 Subject: [PATCH 5/8] Add test for thread suspend/resume API * tests/test.c (tiny_reverse_test): Call GC_gcollect (only if GC_ENABLE_SUSPEND_THREAD). * tests/test.c: Include "javaxfc.h" if GC_ENABLE_SUSPEND_THREAD (and GC_PTHREADS) to GC_suspend/resume_thread declared. * tests/test.c (fork_a_thread): Test GC_is_thread_suspended, GC_suspend_thread, GC_resume_thread (only if the functions defined). --- tests/test.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test.c b/tests/test.c index a8604b843..12b3f9073 100644 --- a/tests/test.c +++ b/tests/test.c @@ -506,10 +506,18 @@ void check_marks_int_list(sexpr x) check_ints(reverse(reverse(ints(1, TINY_REVERSE_UPPER_VALUE))), 1, TINY_REVERSE_UPPER_VALUE); } +# if defined(GC_ENABLE_SUSPEND_THREAD) + /* Force collection from a thread. */ + GC_gcollect(); +# endif return 0; } # if defined(GC_PTHREADS) +# if defined(GC_ENABLE_SUSPEND_THREAD) +# include "javaxfc.h" +# endif + void fork_a_thread(void) { pthread_t t; @@ -518,6 +526,27 @@ void check_marks_int_list(sexpr x) GC_printf("Small thread creation failed %d\n", code); FAIL; } +# if defined(GC_ENABLE_SUSPEND_THREAD) && !defined(GC_DARWIN_THREADS) \ + && !defined(GC_OPENBSD_UTHREADS) && !defined(GC_WIN32_THREADS) \ + && !defined(NACL) + if (GC_is_thread_suspended(t)) { + GC_printf("Running thread should be not suspended\n"); + FAIL; + } + /* Thread could be running or already terminated (but not joined). */ + GC_suspend_thread(t); + if (!GC_is_thread_suspended(t)) { + GC_printf("Thread expected to be suspended\n"); + FAIL; + } + GC_suspend_thread(t); /* should be no-op */ + GC_resume_thread(t); + if (GC_is_thread_suspended(t)) { + GC_printf("Resumed thread should be not suspended\n"); + FAIL; + } + GC_resume_thread(t); /* should be no-op */ +# endif if ((code = pthread_join(t, 0)) != 0) { GC_printf("Small thread join failed %d\n", code); FAIL; From 5327b21e518fd62254e33b135eb5e1d217f9fa77 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Mon, 30 May 2016 22:46:59 +0300 Subject: [PATCH 6/8] Refactoring of android_thread_kill/pthread_kill calls * pthread_stop_world.c (android_thread_kill) [USE_TKILL_ON_ANDROID]: Move definition upper (to be before its first use); remove forward declaration. * pthread_stop_world.c (THREAD_SYSTEM_ID, RAISE_SIGNAL): New macro. * pthread_stop_world.c (GC_suspend_thread): Remove "result" local variable * pthread_stop_world.c (GC_suspend_thread, GC_suspend_all, GC_start_world): Use RAISE_SIGNAL() instead of pthread_kill and android_thread_kill. * pthread_stop_world.c (GC_suspend_all, GC_start_world): Remove "thread_id" local variable; use THREAD_SYSTEM_ID instead of thread_id. --- pthread_stop_world.c | 85 +++++++++++++++----------------------------- 1 file changed, 29 insertions(+), 56 deletions(-) diff --git a/pthread_stop_world.c b/pthread_stop_world.c index 632c8ad4a..31d550860 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -356,6 +356,29 @@ STATIC void GC_restart_handler(int sig) # endif } +# ifdef USE_TKILL_ON_ANDROID + extern int tkill(pid_t tid, int sig); /* from sys/linux-unistd.h */ + + static int android_thread_kill(pid_t tid, int sig) + { + int ret; + int old_errno = errno; + + ret = tkill(tid, sig); + if (ret < 0) { + ret = errno; + errno = old_errno; + } + return ret; + } + +# define THREAD_SYSTEM_ID(t) (t)->kernel_id +# define RAISE_SIGNAL(t, sig) android_thread_kill(THREAD_SYSTEM_ID(t), sig) +# else +# define THREAD_SYSTEM_ID(t) (t)->id +# define RAISE_SIGNAL(t, sig) pthread_kill(THREAD_SYSTEM_ID(t), sig) +# endif /* !USE_TKILL_ON_ANDROID */ + # ifdef GC_ENABLE_SUSPEND_THREAD # ifndef GC_TIME_LIMIT # define GC_TIME_LIMIT 50 @@ -379,13 +402,8 @@ STATIC void GC_restart_handler(int sig) return NULL; } -# ifdef USE_TKILL_ON_ANDROID - static int android_thread_kill(pid_t tid, int sig); -# endif - GC_API void GC_CALL GC_suspend_thread(GC_SUSPEND_THREAD_ID thread) { GC_thread t; - int result; DCL_LOCK_STATE; LOCK(); @@ -395,12 +413,7 @@ STATIC void GC_restart_handler(int sig) if ((pthread_t)thread == pthread_self()) { (void)GC_do_blocking(suspend_self_inner, t); } else { -# ifndef USE_TKILL_ON_ANDROID - result = pthread_kill(t -> id, GC_sig_suspend); -# else - result = android_thread_kill(t -> kernel_id, GC_sig_suspend); -# endif - switch (result) { + switch (RAISE_SIGNAL(t, GC_sig_suspend)) { case ESRCH: case 0: break; @@ -547,24 +560,6 @@ GC_INNER void GC_push_all_stacks(void) int GC_stopping_pid = 0; #endif -#ifdef USE_TKILL_ON_ANDROID - extern int tkill(pid_t tid, int sig); /* from sys/linux-unistd.h */ - - static int android_thread_kill(pid_t tid, int sig) - { - int ret; - int old_errno = errno; - - ret = tkill(tid, sig); - if (ret < 0) { - ret = errno; - errno = old_errno; - } - - return ret; - } -#endif /* USE_TKILL_ON_ANDROID */ - /* We hold the allocation lock. Suspend all threads that might */ /* still be running. Return the number of suspend signals that */ /* were sent. */ @@ -573,11 +568,6 @@ STATIC int GC_suspend_all(void) int n_live_threads = 0; int i; # ifndef NACL -# ifndef USE_TKILL_ON_ANDROID - pthread_t thread_id; -# else - pid_t thread_id; -# endif GC_thread p; # ifndef GC_OPENBSD_UTHREADS int result; @@ -614,13 +604,7 @@ STATIC int GC_suspend_all(void) (void *)p->id); } # else -# ifndef USE_TKILL_ON_ANDROID - thread_id = p -> id; - result = pthread_kill(thread_id, GC_sig_suspend); -# else - thread_id = p -> kernel_id; - result = android_thread_kill(thread_id, GC_sig_suspend); -# endif + result = RAISE_SIGNAL(p, GC_sig_suspend); switch(result) { case ESRCH: /* Not really there anymore. Possible? */ @@ -629,8 +613,8 @@ STATIC int GC_suspend_all(void) case 0: if (GC_on_thread_event) GC_on_thread_event(GC_EVENT_THREAD_SUSPENDED, - (void *)(word)thread_id); - /* Note: thread_id might be truncated. */ + (void *)(word)THREAD_SYSTEM_ID(p)); + /* Note: thread id might be truncated. */ break; default: ABORT_ARG1("pthread_kill failed at suspend", @@ -944,11 +928,6 @@ GC_INNER void GC_start_world(void) register int n_live_threads = 0; register int result; # endif -# ifndef USE_TKILL_ON_ANDROID - pthread_t thread_id; -# else - pid_t thread_id; -# endif # ifdef GC_NETBSD_THREADS_WORKAROUND int code; # endif @@ -978,13 +957,7 @@ GC_INNER void GC_start_world(void) if (GC_on_thread_event) GC_on_thread_event(GC_EVENT_THREAD_UNSUSPENDED, (void *)p->id); # else -# ifndef USE_TKILL_ON_ANDROID - thread_id = p -> id; - result = pthread_kill(thread_id, GC_sig_thr_restart); -# else - thread_id = p -> kernel_id; - result = android_thread_kill(thread_id, GC_sig_thr_restart); -# endif + result = RAISE_SIGNAL(p, GC_sig_thr_restart); switch(result) { case ESRCH: /* Not really there anymore. Possible? */ @@ -993,7 +966,7 @@ GC_INNER void GC_start_world(void) case 0: if (GC_on_thread_event) GC_on_thread_event(GC_EVENT_THREAD_UNSUSPENDED, - (void *)(word)thread_id); + (void *)(word)THREAD_SYSTEM_ID(p)); break; default: ABORT_ARG1("pthread_kill failed at resume", From 59e2bcf96430d5ae4e307ac5d8323bf4ea679e47 Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Mon, 20 Jun 2016 11:38:50 +0300 Subject: [PATCH 7/8] Fix deadlock (and double lock) in explicit thread suspend/resume * pthread_stop_world.c (GC_suspend_handler_inner) [GC_ENABLE_SUSPEND_THREAD]: If SUSPENDED_EXT flag then set stop_info.stack_ptr, call sem_post(suspend_ack_sem), and call suspend_self_inner instead of GC_do_blocking(suspend_self_inner). * pthread_stop_world.c [GC_ENABLE_SUSPEND_THREAD] (GC_suspend_thread): No-op if already suspended; UNLOCK before GC_do_blocking (if self-suspend); add TODO about GC_retry_signals; clear SUSPENDED_EXT flag if RAISE_SIGNAL failed with ESRCH code; sem_wait(suspend_ack_sem) to let the suspend handler to lookup the thread and store stack_ptr (and save registers if needed). * pthread_stop_world.c (GC_suspend_all, GC_start_world): Skip threads with SUSPENDED_EXT flag. --- pthread_stop_world.c | 62 ++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 17 deletions(-) diff --git a/pthread_stop_world.c b/pthread_stop_world.c index 31d550860..244731e48 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -267,8 +267,16 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED, # ifdef GC_ENABLE_SUSPEND_THREAD if ((me -> flags & SUSPENDED_EXT) != 0) { - /* TODO: GC_with_callee_saves_pushed is redundant here. */ - (void)GC_do_blocking(suspend_self_inner, me); +# ifdef SPARC + me -> stop_info.stack_ptr = GC_save_regs_in_stack(); +# else + me -> stop_info.stack_ptr = GC_approx_sp(); +# ifdef IA64 + me -> backing_store_ptr = GC_save_regs_in_stack(); +# endif +# endif + sem_post(&GC_suspend_ack_sem); + suspend_self_inner(me); # ifdef DEBUG_THREADS GC_log_printf("Continuing %p on GC_resume_thread\n", (void *)self); # endif @@ -408,19 +416,39 @@ STATIC void GC_restart_handler(int sig) LOCK(); t = GC_lookup_thread((pthread_t)thread); - if (t != NULL) { - t -> flags |= SUSPENDED_EXT; - if ((pthread_t)thread == pthread_self()) { - (void)GC_do_blocking(suspend_self_inner, t); - } else { - switch (RAISE_SIGNAL(t, GC_sig_suspend)) { - case ESRCH: - case 0: - break; - default: - ABORT("pthread_kill failed"); - } - } + if (t == NULL || (t -> flags & SUSPENDED_EXT) != 0) { + UNLOCK(); + return; + } + + t -> flags |= SUSPENDED_EXT; + if ((pthread_t)thread == pthread_self()) { + UNLOCK(); + /* It is safe as "t" cannot become invalid here (no race with */ + /* GC_unregister_my_thread). */ + (void)GC_do_blocking(suspend_self_inner, t); + return; + } + + /* TODO: Support GC_retry_signals */ + switch (RAISE_SIGNAL(t, GC_sig_suspend)) { + case ESRCH: + /* Not really there anymore. Possible? */ + t -> flags &= ~SUSPENDED_EXT; + UNLOCK(); + return; + case 0: + break; + default: + ABORT("pthread_kill failed"); + } + + /* Wait for the thread to complete threads table lookup and */ + /* stack_ptr assignment. */ + GC_ASSERT(GC_thr_initialized); + while (sem_wait(&GC_suspend_ack_sem) != 0) { + if (errno != EINTR) + ABORT("sem_wait for handler failed (suspend_self)"); } UNLOCK(); } @@ -581,7 +609,7 @@ STATIC int GC_suspend_all(void) for (i = 0; i < THREAD_TABLE_SZ; i++) { for (p = GC_threads[i]; p != 0; p = p -> next) { if (!THREAD_EQUAL(p -> id, self)) { - if (p -> flags & FINISHED) continue; + if ((p -> flags & (FINISHED | SUSPENDED_EXT)) != 0) continue; if (p -> thread_blocked) /* Will wait */ continue; # ifndef GC_OPENBSD_UTHREADS if (p -> stop_info.last_stop_count == GC_stop_count) continue; @@ -942,7 +970,7 @@ GC_INNER void GC_start_world(void) for (i = 0; i < THREAD_TABLE_SZ; i++) { for (p = GC_threads[i]; p != 0; p = p -> next) { if (!THREAD_EQUAL(p -> id, self)) { - if (p -> flags & FINISHED) continue; + if ((p -> flags & (FINISHED | SUSPENDED_EXT)) != 0) continue; if (p -> thread_blocked) continue; # ifndef GC_OPENBSD_UTHREADS n_live_threads++; From c651715cd32958d6c6efe3800eb15f6667d4784f Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Tue, 21 Jun 2016 09:48:21 +0300 Subject: [PATCH 8/8] Fix GC_suspend_thread for terminated threads * pthread_stop_world.c (GC_suspend_thread): Do not clear SUSPENDED_EXT flag in case of RAISE_SIGNAL() failure, add assertion about FINISHED (in case of ESRCH), update comment. * pthread_stop_world.c (GC_register_my_thread): Add assertion that SUSPENDED_EXT flag is not set if the thread is registered from a thread key destructor. --- pthread_stop_world.c | 5 +++-- pthread_support.c | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pthread_stop_world.c b/pthread_stop_world.c index 244731e48..1ee9ee8f9 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -433,8 +433,9 @@ STATIC void GC_restart_handler(int sig) /* TODO: Support GC_retry_signals */ switch (RAISE_SIGNAL(t, GC_sig_suspend)) { case ESRCH: - /* Not really there anymore. Possible? */ - t -> flags &= ~SUSPENDED_EXT; + /* Not really there anymore (terminated but not joined yet). */ + /* No need to wait but leave the suspension flag on. */ + GC_ASSERT((t -> flags & FINISHED) != 0); UNLOCK(); return; case 0: diff --git a/pthread_support.c b/pthread_support.c index d9f935f76..d0c9cb103 100644 --- a/pthread_support.c +++ b/pthread_support.c @@ -1617,6 +1617,7 @@ GC_API int GC_CALL GC_register_my_thread(const struct GC_stack_base *sb) } else if ((me -> flags & FINISHED) != 0) { /* This code is executed when a thread is registered from the */ /* client thread key destructor. */ + GC_ASSERT((me -> flags & SUSPENDED_EXT) == 0); GC_record_stack_base(me, sb); me -> flags &= ~FINISHED; /* but not DETACHED */ # ifdef GC_EXPLICIT_SIGNALS_UNBLOCK