From 78d3d5fb29beb6068fb82a68e680af05108c09bf Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Tue, 9 Feb 2021 23:11:59 -0500 Subject: [PATCH] i#4719 qemu: Add -takeover_timeout_ms (#4728) 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 --- core/optionsx.h | 9 +++++++-- core/unix/os.c | 23 ++++++++++++++--------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/core/optionsx.h b/core/optionsx.h index d97d9e93f19..e83719be1a1 100644 --- a/core/optionsx.h +++ b/core/optionsx.h @@ -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") diff --git a/core/unix/os.c b/core/unix/os.c index 12e451af544..d1be07ae87b 100644 --- a/core/unix/os.c +++ b/core/unix/os.c @@ -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. * *******************************************************************************/ @@ -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. */ @@ -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());