diff --git a/util/fuchsia/scoped_task_suspend.cc b/util/fuchsia/scoped_task_suspend.cc index c5871963..ca0fd564 100644 --- a/util/fuchsia/scoped_task_suspend.cc +++ b/util/fuchsia/scoped_task_suspend.cc @@ -40,80 +40,42 @@ zx_obj_type_t GetHandleType(zx_handle_t handle) { return basic.type; } -enum class SuspensionResult { - FailedSuspendCall, - FailedToSuspendInTimelyFashion, - Succeeded, -}; - -SuspensionResult SuspendThread(zx_handle_t thread) { - zx_status_t status = zx_task_suspend(thread); - ZX_LOG_IF(ERROR, status != ZX_OK, status) << "zx_task_suspend"; - if (status != ZX_OK) - return SuspensionResult::FailedSuspendCall; - // zx_task_suspend() suspends the thread "sometime soon", but it's hard to - // use when it's not guaranteed to be suspended after return. Try reading the - // thread state until the registers are retrievable, which means that the - // thread is actually suspended. Don't wait forever in case the suspend - // failed for whatever reason, but try a few times. - for (int i = 0; i < 5; ++i) { - zx_thread_state_general_regs_t regs; - status = zx_thread_read_state( - thread, ZX_THREAD_STATE_GENERAL_REGS, ®s, sizeof(regs)); - if (status == ZX_OK) { - return SuspensionResult::Succeeded; - } - zx_nanosleep(zx_deadline_after(ZX_MSEC(10))); +// Returns the suspend token of the suspended thread. This function attempts +// to wait a short time for the thread to actually suspend before returning +// but this is not guaranteed. +base::ScopedZxHandle SuspendThread(zx_handle_t thread) { + zx_handle_t token = ZX_HANDLE_INVALID; + zx_status_t status = zx_task_suspend_token(thread, &token); + if (status != ZX_OK) { + ZX_LOG(ERROR, status) << "zx_task_suspend"; + base::ScopedZxHandle(); } - LOG(ERROR) << "thread failed to suspend"; - return SuspensionResult::FailedToSuspendInTimelyFashion; -} -bool ResumeThread(zx_handle_t thread) { - zx_status_t status = zx_task_resume(thread, 0); - ZX_LOG_IF(ERROR, status != ZX_OK, status) << "zx_task_resume"; - return status == ZX_OK; + zx_signals_t observed = 0u; + if (zx_object_wait_one(thread, ZX_THREAD_SUSPENDED, + zx_deadline_after(ZX_MSEC(50)), &observed) != ZX_OK) { + LOG(ERROR) << "thread failed to suspend"; + } + return base::ScopedZxHandle(token); } } // namespace -ScopedTaskSuspend::ScopedTaskSuspend(zx_handle_t task) : task_(task) { - DCHECK_NE(task_, zx_process_self()); - DCHECK_NE(task_, zx_thread_self()); +ScopedTaskSuspend::ScopedTaskSuspend(zx_handle_t task) { + DCHECK_NE(task, zx_process_self()); + DCHECK_NE(task, zx_thread_self()); - zx_obj_type_t type = GetHandleType(task_); + zx_obj_type_t type = GetHandleType(task); if (type == ZX_OBJ_TYPE_THREAD) { - // Note that task_ is only marked invalid if the zx_task_suspend() call - // completely fails, otherwise the suspension might just not have taken - // effect yet, so avoid leaving it suspended forever by still resuming on - // destruction. - if (SuspendThread(task_) == SuspensionResult::FailedSuspendCall) { - task_ = ZX_HANDLE_INVALID; - } + suspend_tokens_.push_back(SuspendThread(task)); } else if (type == ZX_OBJ_TYPE_PROCESS) { - for (const auto& thread : GetChildHandles(task_, ZX_INFO_PROCESS_THREADS)) { - SuspendThread(thread.get()); - } + for (const auto& thread : GetChildHandles(task, ZX_INFO_PROCESS_THREADS)) + suspend_tokens_.push_back(SuspendThread(thread.get())); } else { LOG(ERROR) << "unexpected handle type"; - task_ = ZX_HANDLE_INVALID; } } -ScopedTaskSuspend::~ScopedTaskSuspend() { - if (task_ != ZX_HANDLE_INVALID) { - zx_obj_type_t type = GetHandleType(task_); - if (type == ZX_OBJ_TYPE_THREAD) { - ResumeThread(task_); - } else if (type == ZX_OBJ_TYPE_PROCESS) { - for (const auto& thread : - GetChildHandles(task_, ZX_INFO_PROCESS_THREADS)) { - ResumeThread(thread.get()); - } - } else { - LOG(ERROR) << "unexpected handle type"; - } - } -} +ScopedTaskSuspend::~ScopedTaskSuspend() = default; } // namespace crashpad diff --git a/util/fuchsia/scoped_task_suspend.h b/util/fuchsia/scoped_task_suspend.h index d3790c8c..f817364d 100644 --- a/util/fuchsia/scoped_task_suspend.h +++ b/util/fuchsia/scoped_task_suspend.h @@ -17,20 +17,21 @@ #include +#include + +#include "base/fuchsia/scoped_zx_handle.h" #include "base/macros.h" namespace crashpad { //! \brief Manages the suspension of another task. //! -//! Currently, suspends and resumes are not counted on Fuchsia, so while this -//! class attempts to manage suspension of a task, if another caller or process -//! is simultaneously suspending or resuming this task, the results may not be -//! as expected. +//! The underlying API only supports suspending threads (despite its name) not +//! entire tasks. As a result, it's possible some threads may not be correctly +//! suspended/resumed as their creation might race enumeration. //! -//! Additionally, the underlying API only supports suspending threads (despite -//! its name) not entire tasks. As a result, it's possible some threads may not -//! be correctly suspended/resumed as their creation might race enumeration. +//! Additionally, suspending a thread is asynchronous and may take an +//! arbitrary amount of time. //! //! Because of these limitations, this class is limited to being a best-effort, //! and correct suspension/resumption cannot be relied upon. @@ -43,7 +44,8 @@ class ScopedTaskSuspend { ~ScopedTaskSuspend(); private: - zx_handle_t task_; // weak + // Could be one (for a thread) or many (for every process in a thread). + std::vector suspend_tokens_; DISALLOW_COPY_AND_ASSIGN(ScopedTaskSuspend); };