Skip to content

Commit

Permalink
Do not resend the restart signal to threads that are already restarted
Browse files Browse the repository at this point in the history
(fix of commit 3498427)

Issue #181 (bdwgc).

* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL] (GC_stop_count):
Update comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Add assertion that my_stop_count is even.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): Mask lowest bit of last_stop_count when
checking for the duplicate signal.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_handler_inner): If GC_retry_signals then
set the lowest bit of last_stop_count (by AO_store_release) after the
second sem_post() call; add comment.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_suspend_all): Add comment for last_stop_count check.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL] (GC_stop_world):
Increment GC_stop_count by 2 (instead of by one).
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_restart_all): If GC_retry_signals and last_stop_count has the same
value as GC_stop_count+1 then do not increment n_live_threads and do
not send the restart signal to the thread.
  • Loading branch information
ivmai committed Apr 4, 2018
1 parent 4aaabd8 commit 295a2f2
Showing 1 changed file with 27 additions and 13 deletions.
40 changes: 27 additions & 13 deletions pthread_stop_world.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ STATIC void GC_remove_allowed_signals(sigset_t *set)
static sigset_t suspend_handler_mask;

STATIC volatile AO_t GC_stop_count = 0;
/* Incremented at the beginning of GC_stop_world. */
/* Incremented by two at the beginning of */
/* GC_stop_world (the lowest bit is always 0). */

STATIC volatile AO_t GC_world_is_stopped = FALSE;
/* FALSE ==> it is safe for threads to restart, */
Expand Down Expand Up @@ -303,6 +304,7 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
# ifdef DEBUG_THREADS
GC_log_printf("Suspending %p\n", (void *)self);
# endif
GC_ASSERT(((word)my_stop_count & 1) == 0);

me = GC_lookup_thread_async(self);

Expand All @@ -319,7 +321,8 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
}
# endif

if (me -> stop_info.last_stop_count == my_stop_count) {
if (((word)me->stop_info.last_stop_count & ~(word)0x1)
== (word)my_stop_count) {
/* Duplicate signal. OK if we are retrying. */
if (!GC_retry_signals) {
WARN("Duplicate suspend signal in thread %p\n", self);
Expand Down Expand Up @@ -367,17 +370,25 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
# ifdef DEBUG_THREADS
GC_log_printf("Continuing %p\n", (void *)self);
# endif
# ifdef GC_NETBSD_THREADS_WORKAROUND
# ifndef GC_NETBSD_THREADS_WORKAROUND
if (GC_retry_signals)
# endif
{
/* If the RESTART signal loss is possible (though it should be */
/* less likely than losing the SUSPEND signal as we do not do */
/* much between the first sem_post and sigsuspend calls), more */
/* handshaking is provided to work around it. */
sem_post(&GC_suspend_ack_sem);
# else
if (GC_retry_signals) {
/* If the RESTART signal loss is possible (though it should be */
/* less likely than losing the SUSPEND signal as we do not do */
/* much between the sem_post and sigsuspend calls), more */
/* handshaking is provided to work around it. */
sem_post(&GC_suspend_ack_sem);
# ifdef GC_NETBSD_THREADS_WORKAROUND
if (GC_retry_signals)
# endif
{
/* Set the flag (the lowest bit of last_stop_count) that the */
/* thread has been restarted. */
AO_store_release(&me->stop_info.last_stop_count,
(AO_t)((word)my_stop_count | 1));
}
# endif
}
RESTORE_CANCEL(cancel_state);
}

Expand Down Expand Up @@ -742,7 +753,7 @@ STATIC int GC_suspend_all(void)
if (p -> suspended_ext) continue;
# endif
if (AO_load(&p->stop_info.last_stop_count) == GC_stop_count)
continue;
continue; /* matters only if GC_retry_signals */
n_live_threads++;
# endif
# ifdef DEBUG_THREADS
Expand Down Expand Up @@ -863,7 +874,7 @@ GC_INNER void GC_stop_world(void)
# if defined(GC_OPENBSD_UTHREADS) || defined(NACL)
(void)GC_suspend_all();
# else
AO_store(&GC_stop_count, GC_stop_count+1);
AO_store(&GC_stop_count, (AO_t)((word)GC_stop_count + 2));
/* Only concurrent reads are possible. */
AO_store_release(&GC_world_is_stopped, TRUE);
n_live_threads = GC_suspend_all();
Expand Down Expand Up @@ -1058,6 +1069,9 @@ GC_INNER void GC_stop_world(void)
# ifdef GC_ENABLE_SUSPEND_THREAD
if (p -> suspended_ext) continue;
# endif
if (GC_retry_signals && AO_load(&p->stop_info.last_stop_count)
== (AO_t)((word)GC_stop_count | 1))
continue; /* The thread has been restarted. */
n_live_threads++;
# endif
# ifdef DEBUG_THREADS
Expand Down

0 comments on commit 295a2f2

Please sign in to comment.