Skip to content

Commit

Permalink
i#3023: add new-thread barrier during attach (#3024)
Browse files Browse the repository at this point in the history
Adds a barrier to taken-over threads creating new threads until attach
is fully finished.  This avoids problems keeping up with an app that
is creating threads while we try to attach.

Fixes #3023
  • Loading branch information
derekbruening authored and fhahn committed Jun 18, 2018
1 parent 74d9700 commit 23f6268
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 5 deletions.
8 changes: 7 additions & 1 deletion core/dynamo.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ bool post_execve = false;
byte * initstack;

event_t dr_app_started;
event_t dr_attach_finished;

#ifdef WINDOWS
/* PR203701: separate stack for error reporting when the dstack is exhausted */
Expand Down Expand Up @@ -649,6 +650,8 @@ dynamorio_app_init(void)
#endif
jitopt_init();

dr_attach_finished = create_broadcast_event();

/* New client threads rely on dr_app_started being initialized, so do
* that before initializing clients.
*/
Expand Down Expand Up @@ -1016,6 +1019,7 @@ dynamo_shared_exit(thread_record_t *toexit /* must ==cur thread for Linux */
dynamo_exited_and_cleaned = true;

destroy_event(dr_app_started);
destroy_event(dr_attach_finished);

/* we want dcontext around for loader_exit() */
if (get_thread_private_dcontext() != NULL)
Expand Down Expand Up @@ -2850,7 +2854,7 @@ dynamo_thread_not_under_dynamo(dcontext_t *dcontext)
#endif
}

#define MAX_TAKE_OVER_ATTEMPTS 4
#define MAX_TAKE_OVER_ATTEMPTS 8

/* Mark this thread as under DR, and take over other threads in the current process.
*/
Expand Down Expand Up @@ -2879,6 +2883,8 @@ dynamorio_take_over_threads(dcontext_t *dcontext)
bb_lock_start = true;
} while (found_threads && attempts < MAX_TAKE_OVER_ATTEMPTS);
os_process_under_dynamorio_complete(dcontext);
/* End the barrier to new threads. */
signal_event(dr_attach_finished);

if (found_threads) {
REPORT_FATAL_ERROR_AND_EXIT(dcontext, FAILED_TO_TAKE_OVER_THREADS,
Expand Down
1 change: 1 addition & 0 deletions core/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,7 @@ extern bool dynamo_all_threads_synched; /* are all other threads suspended safel
extern bool doing_detach;

extern event_t dr_app_started;
extern event_t dr_attach_finished;

#if defined(CLIENT_INTERFACE) || defined(STANDALONE_UNIT_TEST)
extern bool standalone_library; /* used as standalone library */
Expand Down
23 changes: 19 additions & 4 deletions core/unix/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -2485,6 +2485,18 @@ os_swap_dr_tls(dcontext_t *dcontext, bool to_app)
#endif
}

static void
os_new_thread_pre(void)
{
/* We use a barrier on new threads to ensure we make progress when
* attaching to an app that is continually making threads.
* XXX i#1305: if we fully suspend all threads during attach we can
* get rid of this barrier.
*/
wait_for_event(dr_attach_finished, 0);
ATOMIC_INC(int, uninit_thread_count);
}

static void
os_clone_pre(dcontext_t *dcontext)
{
Expand Down Expand Up @@ -7065,7 +7077,7 @@ pre_system_call(dcontext_t *dcontext)
if (is_thread_create_syscall(dcontext)) {
create_clone_record(dcontext, sys_param_addr(dcontext, 1) /*newsp*/);
os_clone_pre(dcontext);
ATOMIC_INC(int, uninit_thread_count);
os_new_thread_pre();
} else /* This is really a fork. */
os_fork_pre(dcontext);
break;
Expand All @@ -7088,7 +7100,7 @@ pre_system_call(dcontext_t *dcontext)
dcontext->sys_param1 = (reg_t) func_arg;
*sys_param_addr(dcontext, 0) = (reg_t) new_bsdthread_intercept;
*sys_param_addr(dcontext, 1) = (reg_t) clone_rec;
ATOMIC_INC(int, uninit_thread_count);
os_new_thread_pre();
break;
}
case SYS_posix_spawn: {
Expand Down Expand Up @@ -7121,7 +7133,7 @@ pre_system_call(dcontext_t *dcontext)
create_clone_record(dcontext, (reg_t *)&mc->xsp /*child uses parent sp*/);
# endif
os_clone_pre(dcontext);
ATOMIC_INC(int, uninit_thread_count);
os_new_thread_pre();
break;
}
#endif
Expand Down Expand Up @@ -9678,7 +9690,10 @@ signal_event(event_t e)
{
mutex_lock(&e->lock);
ksynch_set_value(&e->signaled, 1);
ksynch_wake(&e->signaled);
if (e->broadcast)
ksynch_wake_all(&e->signaled);
else
ksynch_wake(&e->signaled);
LOG(THREAD_GET, LOG_THREADS, 3,
"thread "TIDFMT" signalling event "PFX"\n",get_thread_id(),e);
mutex_unlock(&e->lock);
Expand Down

0 comments on commit 23f6268

Please sign in to comment.