Skip to content

Commit

Permalink
i#4719 qemu: Add -takeover_timeout_ms (#4728)
Browse files Browse the repository at this point in the history
Fixes a problem with the hardcoded small timeout from PR #4725 by
parameterizing the timeout in a new option -takeover_timeout_ms.  It
is set to a high value by default; the plan is to have -xarch_root set
it to a low value for the common QEMU case of running a small test,
while still overridable for large apps.

Renames -ignore_takeover_timeout to -unsafe_ignore_takeover_timeout to
indicate that it can cause problems if actual application threads are
left native.

Issue: #4719
  • Loading branch information
derekbruening authored Feb 10, 2021
1 parent 79c8fb0 commit 78d3d5f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 11 deletions.
9 changes: 7 additions & 2 deletions core/optionsx.h
Original file line number Diff line number Diff line change
Expand Up @@ -3200,8 +3200,13 @@ OPTION(bool, multi_thread_exit,
"do not guarantee that process exit event callback is invoked single-threaded")
OPTION(bool, skip_thread_exit_at_exit, "skip thread exit events at process exit")
#endif
OPTION(bool, ignore_takeover_timeout,
"ignore timeouts trying to take over one or more threads when initializing")
OPTION(bool, unsafe_ignore_takeover_timeout,
"ignore timeouts trying to take over one or more threads when initializing, "
"leaving those threads native, which is potentially unsafe")
OPTION_DEFAULT(uint, takeover_timeout_ms, 30000,
"timeout in milliseconds for each thread when taking over at "
"initialization/attach. Reaching a timeout is fatal, unless "
"-unsafe_ignore_takeover_timeout is set.")

#ifdef EXPOSE_INTERNAL_OPTIONS
OPTION_NAME(bool, optimize, " synthethic", "set if ANY opts are on")
Expand Down
23 changes: 14 additions & 9 deletions core/unix/os.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* *******************************************************************************
* Copyright (c) 2010-2020 Google, Inc. All rights reserved.
* Copyright (c) 2010-2021 Google, Inc. All rights reserved.
* Copyright (c) 2011 Massachusetts Institute of Technology All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* *******************************************************************************/
Expand Down Expand Up @@ -10154,8 +10154,11 @@ os_take_over_all_unknown_threads(dcontext_t *dcontext)
SYSLOG(SYSLOG_VERBOSE, INFO_ATTACHED, 3, buf, get_application_name(),
get_application_pid());
}
/* We split the wait up so that we'll break early on an exited thread. */
static const int wait_ms = 25;
static const int max_attempts = 16;
int max_attempts =
/* Integer division rounding down is fine since we always wait 25ms. */
DYNAMO_OPTION(takeover_timeout_ms) / wait_ms;
int attempts = 0;
while (!wait_for_event(records[i].event, wait_ms)) {
/* The thread may have exited (i#2601). We assume no tid re-use. */
Expand All @@ -10167,15 +10170,17 @@ os_take_over_all_unknown_threads(dcontext_t *dcontext)
break;
}
if (++attempts > max_attempts) {
if (DYNAMO_OPTION(ignore_takeover_timeout)) {
SYSLOG(SYSLOG_VERBOSE, THREAD_TAKEOVER_TIMED_OUT, 3,
get_application_name(), get_application_pid(),
"Continuing since -ignore_takeover_timeout is set.");
if (DYNAMO_OPTION(unsafe_ignore_takeover_timeout)) {
SYSLOG(
SYSLOG_VERBOSE, THREAD_TAKEOVER_TIMED_OUT, 3,
get_application_name(), get_application_pid(),
"Continuing since -unsafe_ignore_takeover_timeout is set.");
++threads_timed_out;
} else {
SYSLOG(SYSLOG_VERBOSE, THREAD_TAKEOVER_TIMED_OUT, 3,
get_application_name(), get_application_pid(),
"Aborting. Use -ignore_takeover_timeout to ignore.");
SYSLOG(
SYSLOG_VERBOSE, THREAD_TAKEOVER_TIMED_OUT, 3,
get_application_name(), get_application_pid(),
"Aborting. Use -unsafe_ignore_takeover_timeout to ignore.");
REPORT_FATAL_ERROR_AND_EXIT(FAILED_TO_TAKE_OVER_THREADS, 2,
get_application_name(),
get_application_pid());
Expand Down

0 comments on commit 78d3d5f

Please sign in to comment.