From 8297b19a5e85d7c4a437b4601c7007bda8273758 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Fri, 14 Apr 2017 14:50:11 -0400 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20attempt=20to=20do=20periodic=20?= =?UTF-8?q?tasks=20in=20a=20secondary=20crashpad=5Fhandler?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 76a67a37b1d0 adds crashpad_handler’s --monitor-self argument, which results in a second crashpad_handler instance running out of the same database as the initial crashpad_handler instance that it monitors. The two handlers start at nearly the same time, and will initially be on precisely the same schedule for periodic tasks such as scanning for new reports to upload and pruning the database. This is an unnecessary duplication of effort. This adds a new --no-periodic-tasks argument to crashpad_handler. When the first instance of crashpad_handler starts a second to monitor it, it will use this argument, which prevents the second instance from performing these tasks. When --no-periodic-tasks is in effect, crashpad_handler will still be able to upload crash reports that it knows about by virtue of having written them itself, but it will not scan the database for other pending reports to upload. Bug: crashpad:143 Test: crashpad_util_test ThreadSafeVector.ThreadSafeVector Change-Id: I7b249dd7b6d5782448d8071855818f986b98ab5a Reviewed-on: https://chromium-review.googlesource.com/473827 Reviewed-by: Robert Sesek --- handler/crash_report_upload_thread.cc | 54 +++++++++-- handler/crash_report_upload_thread.h | 40 +++++--- handler/crashpad_handler.md | 17 +++- handler/handler_main.cc | 30 ++++-- handler/mac/crash_report_exception_handler.cc | 2 +- handler/win/crash_report_exception_handler.cc | 2 +- util/stdlib/thread_safe_vector.h | 63 +++++++++++++ util/stdlib/thread_safe_vector_test.cc | 91 +++++++++++++++++++ util/synchronization/semaphore.h | 9 +- util/synchronization/semaphore_mac.cc | 8 ++ util/synchronization/semaphore_posix.cc | 6 ++ util/synchronization/semaphore_test.cc | 12 +++ util/synchronization/semaphore_win.cc | 7 ++ util/thread/worker_thread.cc | 1 - util/thread/worker_thread.h | 11 ++- util/util.gyp | 1 + util/util_test.gyp | 1 + 17 files changed, 323 insertions(+), 32 deletions(-) create mode 100644 util/stdlib/thread_safe_vector.h create mode 100644 util/stdlib/thread_safe_vector_test.cc diff --git a/handler/crash_report_upload_thread.cc b/handler/crash_report_upload_thread.cc index 9ba3f1f8..c13bbf4b 100644 --- a/handler/crash_report_upload_thread.cc +++ b/handler/crash_report_upload_thread.cc @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -29,7 +30,6 @@ #include "snapshot/module_snapshot.h" #include "util/file/file_reader.h" #include "util/misc/metrics.h" -#include "util/misc/uuid.h" #include "util/net/http_body.h" #include "util/net/http_multipart_builder.h" #include "util/net/http_transport.h" @@ -139,15 +139,19 @@ class CallRecordUploadAttempt { CrashReportUploadThread::CrashReportUploadThread(CrashReportDatabase* database, const std::string& url, + bool watch_pending_reports, bool rate_limit, bool upload_gzip) : url_(url), - // Check for pending reports every 15 minutes, even in the absence of a - // signal from the handler thread. This allows for failed uploads to be - // retried periodically, and for pending reports written by other - // processes to be recognized. - thread_(15 * 60, this), + // When watching for pending reports, check every 15 minutes, even in the + // absence of a signal from the handler thread. This allows for failed + // uploads to be retried periodically, and for pending reports written by + // other processes to be recognized. + thread_(watch_pending_reports ? 15 * 60.0 : WorkerThread::kIndefiniteWait, + this), + known_pending_report_uuids_(), database_(database), + watch_pending_reports_(watch_pending_reports), rate_limit_(rate_limit), upload_gzip_(upload_gzip) { } @@ -156,18 +160,43 @@ CrashReportUploadThread::~CrashReportUploadThread() { } void CrashReportUploadThread::Start() { - thread_.Start(0); + thread_.Start(watch_pending_reports_ ? 0.0 : WorkerThread::kIndefiniteWait); } void CrashReportUploadThread::Stop() { thread_.Stop(); } -void CrashReportUploadThread::ReportPending() { +void CrashReportUploadThread::ReportPending(const UUID& report_uuid) { + known_pending_report_uuids_.PushBack(report_uuid); thread_.DoWorkNow(); } void CrashReportUploadThread::ProcessPendingReports() { + std::vector known_report_uuids = known_pending_report_uuids_.Drain(); + for (const UUID& report_uuid : known_report_uuids) { + CrashReportDatabase::Report report; + if (database_->LookUpCrashReport(report_uuid, &report) != + CrashReportDatabase::kNoError) { + continue; + } + + ProcessPendingReport(report); + + // Respect Stop() being called after at least one attempt to process a + // report. + if (!thread_.is_running()) { + return; + } + } + + // Known pending reports are always processed (above). The rest of this + // function is concerned with scanning for pending reports not already known + // to this thread. + if (!watch_pending_reports_) { + return; + } + std::vector reports; if (database_->GetPendingReports(&reports) != CrashReportDatabase::kNoError) { // The database is sick. It might be prudent to stop trying to poke it from @@ -178,6 +207,15 @@ void CrashReportUploadThread::ProcessPendingReports() { } for (const CrashReportDatabase::Report& report : reports) { + if (std::find(known_report_uuids.begin(), + known_report_uuids.end(), + report.uuid) != known_report_uuids.end()) { + // An attempt to process the report already occurred above. The report is + // still pending, so upload must have failed. Don’t retry it immediately, + // it can wait until at least the next pass through this method. + continue; + } + ProcessPendingReport(report); // Respect Stop() being called after at least one attempt to process a diff --git a/handler/crash_report_upload_thread.h b/handler/crash_report_upload_thread.h index 14debacd..c769efed 100644 --- a/handler/crash_report_upload_thread.h +++ b/handler/crash_report_upload_thread.h @@ -20,6 +20,8 @@ #include "base/macros.h" #include "client/crash_report_database.h" +#include "util/misc/uuid.h" +#include "util/stdlib/thread_safe_vector.h" #include "util/thread/worker_thread.h" namespace crashpad { @@ -32,22 +34,28 @@ namespace crashpad { //! report has been added to the database by calling ReportPending(). //! //! Independently of being triggered by ReportPending(), objects of this class -//! periodically examine the database for pending reports. This allows failed -//! upload attempts for reports left in the pending state to be retried. It also -//! catches reports that are added without a ReportPending() signal being -//! caught. This may happen if crash reports are added to the database by other -//! processes. +//! can periodically examine the database for pending reports. This allows +//! failed upload attempts for reports left in the pending state to be retried. +//! It also catches reports that are added without a ReportPending() signal +//! being caught. This may happen if crash reports are added to the database by +//! other processes. class CrashReportUploadThread : public WorkerThread::Delegate { public: //! \brief Constructs a new object. //! //! \param[in] database The database to upload crash reports from. //! \param[in] url The URL of the server to upload crash reports to. + //! \param[in] watch_pending_reports Whether to periodically check for new + //! pending reports not already known to exist. When `false`, only an + //! initial upload attempt will be made for reports known to exist by + //! having been added by the ReportPending() method. No scans for new + //! pending reports will be conducted. //! \param[in] rate_limit Whether uploads should be throttled to a (currently //! hardcoded) rate. //! \param[in] upload_gzip Whether uploads should use `gzip` compression. CrashReportUploadThread(CrashReportDatabase* database, const std::string& url, + bool watch_pending_reports, bool rate_limit, bool upload_gzip); ~CrashReportUploadThread(); @@ -75,8 +83,11 @@ class CrashReportUploadThread : public WorkerThread::Delegate { //! \brief Informs the upload thread that a new pending report has been added //! to the database. //! + //! \param[in] report_uuid The unique identifier of the newly added pending + //! report. + //! //! This method may be called from any thread. - void ReportPending(); + void ReportPending(const UUID& report_uuid); private: //! \brief The result code from UploadReport(). @@ -99,8 +110,13 @@ class CrashReportUploadThread : public WorkerThread::Delegate { kRetry, }; - //! \brief Obtains all pending reports from the database, and calls - //! ProcessPendingReport() to process each one. + //! \brief Calls ProcessPendingReport() on pending reports. + //! + //! Assuming Stop() has not been called, this will process reports that the + //! object has been made aware of in ReportPending(). Additionally, if the + //! object was constructed with \a watch_pending_reports, it will also scan + //! the crash report database for other pending reports, and process those as + //! well. void ProcessPendingReports(); //! \brief Processes a single pending report from the database. @@ -137,11 +153,13 @@ class CrashReportUploadThread : public WorkerThread::Delegate { //! been called on any thread, as well as periodically on a timer. void DoWork(const WorkerThread* thread) override; - std::string url_; + const std::string url_; WorkerThread thread_; + ThreadSafeVector known_pending_report_uuids_; CrashReportDatabase* database_; // weak - bool rate_limit_; - bool upload_gzip_; + const bool watch_pending_reports_; + const bool rate_limit_; + const bool upload_gzip_; DISALLOW_COPY_AND_ASSIGN(CrashReportUploadThread); }; diff --git a/handler/crashpad_handler.md b/handler/crashpad_handler.md index e76fed76..9cb29e43 100644 --- a/handler/crashpad_handler.md +++ b/handler/crashpad_handler.md @@ -151,7 +151,8 @@ establish the Crashpad client environment before running a program. become a client of the second one. The second instance will be started with the same **--annotation**, **--database**, **--monitor-self-annotation**, **--no-rate-limit**, **--no-upload-gzip**, and **--url** arguments as the - original one. The second instance will not be started with a + original one. The second instance will always be started with a + **--no-periodic-tasks** argument, and will not be started with a **--metrics-dir** argument even if the original instance was. Where supported by the underlying operating system, the second instance will @@ -183,6 +184,20 @@ establish the Crashpad client environment before running a program. To prevent excessive accumulation of handler processes, _ARGUMENT_ must not be `--monitor-self`. + * **--no-periodic-tasks** + + Do not scan for new pending crash reports or prune the crash report database. + Only crash reports recorded by this instance of the Crashpad handler will + become eligible for upload in this instance, and only a single initial upload + attempt will be made. + + This option is not intended for general use. It is provided to prevent + multiple instances of the Crashpad handler from duplicating the effort of + performing the same periodic tasks. In normal use, the first instance of the + Crashpad handler will assume the responsibility for performing these tasks, + and will provide this argument to any second instance. See + **--monitor-self**. + * **--no-rate-limit** Do not rate limit the upload of crash reports. By default uploads are diff --git a/handler/handler_main.cc b/handler/handler_main.cc index 641bf0f6..b4a08c51 100644 --- a/handler/handler_main.cc +++ b/handler/handler_main.cc @@ -109,6 +109,7 @@ void Usage(const base::FilePath& me) { " set a module annotation in the handler\n" " --monitor-self-argument=ARGUMENT\n" " provide additional arguments to the second handler\n" +" --no-periodic-tasks don't scan for new reports or prune the database\n" " --no-rate-limit don't rate limit crash uploads\n" " --no-upload-gzip don't use gzip compression when uploading\n" #if defined(OS_WIN) @@ -142,6 +143,7 @@ struct Options { InitialClientData initial_client_data; #endif // OS_MACOSX bool monitor_self; + bool periodic_tasks; bool rate_limit; bool upload_gzip; }; @@ -353,6 +355,7 @@ void MonitorSelf(const Options& options) { return; } std::vector extra_arguments(options.monitor_self_arguments); + extra_arguments.push_back("--no-periodic-tasks"); if (!options.rate_limit) { extra_arguments.push_back("--no-rate-limit"); } @@ -416,6 +419,7 @@ int HandlerMain(int argc, kOptionMonitorSelf, kOptionMonitorSelfAnnotation, kOptionMonitorSelfArgument, + kOptionNoPeriodicTasks, kOptionNoRateLimit, kOptionNoUploadGzip, #if defined(OS_WIN) @@ -456,6 +460,7 @@ int HandlerMain(int argc, required_argument, nullptr, kOptionMonitorSelfArgument}, + {"no-periodic-tasks", no_argument, nullptr, kOptionNoPeriodicTasks}, {"no-rate-limit", no_argument, nullptr, kOptionNoRateLimit}, {"no-upload-gzip", no_argument, nullptr, kOptionNoUploadGzip}, #if defined(OS_WIN) @@ -477,6 +482,7 @@ int HandlerMain(int argc, #if defined(OS_MACOSX) options.handshake_fd = -1; #endif + options.periodic_tasks = true; options.rate_limit = true; options.upload_gzip = true; @@ -540,6 +546,10 @@ int HandlerMain(int argc, options.monitor_self_arguments.push_back(optarg); break; } + case kOptionNoPeriodicTasks: { + options.periodic_tasks = false; + break; + } case kOptionNoRateLimit: { options.rate_limit = false; break; @@ -721,13 +731,19 @@ int HandlerMain(int argc, // TODO(scottmg): options.rate_limit should be removed when we have a // configurable database setting to control upload limiting. // See https://crashpad.chromium.org/bug/23. - CrashReportUploadThread upload_thread( - database.get(), options.url, options.rate_limit, options.upload_gzip); + CrashReportUploadThread upload_thread(database.get(), + options.url, + options.periodic_tasks, + options.rate_limit, + options.upload_gzip); upload_thread.Start(); - PruneCrashReportThread prune_thread(database.get(), - PruneCondition::GetDefault()); - prune_thread.Start(); + std::unique_ptr prune_thread; + if (options.periodic_tasks) { + prune_thread.reset(new PruneCrashReportThread( + database.get(), PruneCondition::GetDefault())); + prune_thread->Start(); + } CrashReportExceptionHandler exception_handler(database.get(), &upload_thread, @@ -744,7 +760,9 @@ int HandlerMain(int argc, exception_handler_server.Run(&exception_handler); upload_thread.Stop(); - prune_thread.Stop(); + if (prune_thread) { + prune_thread->Stop(); + } return EXIT_SUCCESS; } diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index a96131cd..7d3d6297 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -190,7 +190,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( return KERN_FAILURE; } - upload_thread_->ReportPending(); + upload_thread_->ReportPending(uuid); } if (client_options.system_crash_reporter_forwarding != TriState::kDisabled && diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc index 7828aac6..0ab206c1 100644 --- a/handler/win/crash_report_exception_handler.cc +++ b/handler/win/crash_report_exception_handler.cc @@ -130,7 +130,7 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException( return termination_code; } - upload_thread_->ReportPending(); + upload_thread_->ReportPending(uuid); } Metrics::ExceptionCaptureResult(Metrics::CaptureResult::kSuccess); diff --git a/util/stdlib/thread_safe_vector.h b/util/stdlib/thread_safe_vector.h new file mode 100644 index 00000000..f97024d0 --- /dev/null +++ b/util/stdlib/thread_safe_vector.h @@ -0,0 +1,63 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_UTIL_STDLIB_THREAD_SAFE_VECTOR_H_ +#define CRASHPAD_UTIL_STDLIB_THREAD_SAFE_VECTOR_H_ + +#include +#include + +#include "base/macros.h" +#include "base/synchronization/lock.h" + +namespace crashpad { + +//! \brief A wrapper for a `std::vector<>` that can be accessed safely from +//! multiple threads. +//! +//! This is not a drop-in replacement for `std::vector<>`. Only necessary +//! operations are defined. +template +class ThreadSafeVector { + public: + ThreadSafeVector() : vector_(), lock_() {} + ~ThreadSafeVector() {} + + //! \brief Wraps `std::vector<>::%push_back()`. + void PushBack(const T& element) { + base::AutoLock lock_owner(lock_); + vector_.push_back(element); + } + + //! \brief Atomically clears the underlying vector and returns its previous + //! contents. + std::vector Drain() { + std::vector contents; + { + base::AutoLock lock_owner(lock_); + std::swap(vector_, contents); + } + return contents; + } + + private: + std::vector vector_; + base::Lock lock_; + + DISALLOW_COPY_AND_ASSIGN(ThreadSafeVector); +}; + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_STDLIB_THREAD_SAFE_VECTOR_H_ diff --git a/util/stdlib/thread_safe_vector_test.cc b/util/stdlib/thread_safe_vector_test.cc new file mode 100644 index 00000000..805360f4 --- /dev/null +++ b/util/stdlib/thread_safe_vector_test.cc @@ -0,0 +1,91 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/stdlib/thread_safe_vector.h" + +#include "gtest/gtest.h" +#include "util/thread/thread.h" + +namespace crashpad { +namespace test { +namespace { + +constexpr int kElementsPerThread = 100; + +class ThreadSafeVectorTestThread : public Thread { + public: + ThreadSafeVectorTestThread() : thread_safe_vector_(nullptr), start_(0) {} + ~ThreadSafeVectorTestThread() {} + + void SetTestParameters(ThreadSafeVector* thread_safe_vector, int start) { + thread_safe_vector_ = thread_safe_vector; + start_ = start; + } + + // Thread: + void ThreadMain() override { + for (int i = start_; i < start_ + kElementsPerThread; ++i) { + thread_safe_vector_->PushBack(i); + } + } + + private: + ThreadSafeVector* thread_safe_vector_; + int start_; + + DISALLOW_COPY_AND_ASSIGN(ThreadSafeVectorTestThread); +}; + +TEST(ThreadSafeVector, ThreadSafeVector) { + ThreadSafeVector thread_safe_vector; + std::vector vector = thread_safe_vector.Drain(); + EXPECT_TRUE(vector.empty()); + + ThreadSafeVectorTestThread threads[100]; + for (size_t index = 0; index < arraysize(threads); ++index) { + threads[index].SetTestParameters( + &thread_safe_vector, static_cast(index * kElementsPerThread)); + } + + for (size_t index = 0; index < arraysize(threads); ++index) { + threads[index].Start(); + + if (index % 10 == 0) { + // Drain the vector periodically to test that simultaneous Drain() and + // PushBack() operations work properly. + std::vector drained = thread_safe_vector.Drain(); + vector.insert(vector.end(), drained.begin(), drained.end()); + } + } + + for (ThreadSafeVectorTestThread& thread : threads) { + thread.Join(); + } + + std::vector drained = thread_safe_vector.Drain(); + vector.insert(vector.end(), drained.begin(), drained.end()); + bool found[arraysize(threads) * kElementsPerThread] = {}; + EXPECT_EQ(vector.size(), arraysize(found)); + for (int element : vector) { + EXPECT_FALSE(found[element]) << element; + found[element] = true; + } + + vector = thread_safe_vector.Drain(); + EXPECT_TRUE(vector.empty()); +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/util/synchronization/semaphore.h b/util/synchronization/semaphore.h index ac77cb7c..49479639 100644 --- a/util/synchronization/semaphore.h +++ b/util/synchronization/semaphore.h @@ -15,6 +15,8 @@ #ifndef CRASHPAD_UTIL_SYNCHRONIZATION_SEMAPHORE_H_ #define CRASHPAD_UTIL_SYNCHRONIZATION_SEMAPHORE_H_ +#include + #include "build/build_config.h" #if defined(OS_MACOSX) @@ -30,6 +32,10 @@ namespace crashpad { //! \brief An anonymous in-process counting sempahore. class Semaphore { public: + //! \brief A TimedWait() argument that causes an indefinite wait. + static constexpr double kIndefiniteWait = + std::numeric_limits::infinity(); + //! \brief Initializes the semaphore. //! //! \param[in] value The initial value of the semaphore. @@ -51,7 +57,8 @@ class Semaphore { //! \brief Performs a timed wait (or “procure”) operation on the semaphore. //! //! \param[in] seconds The maximum number of seconds to wait for the operation - //! to complete. + //! to complete. If \a seconds is #kIndefiniteWait, this method behaves as + //! Wait(), and will not time out. //! //! \return `false` if the wait timed out, `true` otherwise. //! diff --git a/util/synchronization/semaphore_mac.cc b/util/synchronization/semaphore_mac.cc index e8a79ab4..4f3bf00a 100644 --- a/util/synchronization/semaphore_mac.cc +++ b/util/synchronization/semaphore_mac.cc @@ -14,6 +14,8 @@ #include "util/synchronization/semaphore.h" +#include + #include "base/logging.h" namespace crashpad { @@ -33,6 +35,12 @@ void Semaphore::Wait() { bool Semaphore::TimedWait(double seconds) { DCHECK_GE(seconds, 0.0); + + if (std::isinf(seconds)) { + Wait(); + return true; + } + const dispatch_time_t timeout = dispatch_time(DISPATCH_TIME_NOW, seconds * NSEC_PER_SEC); return dispatch_semaphore_wait(semaphore_, timeout) == 0; diff --git a/util/synchronization/semaphore_posix.cc b/util/synchronization/semaphore_posix.cc index 973f0a5d..c781e4c6 100644 --- a/util/synchronization/semaphore_posix.cc +++ b/util/synchronization/semaphore_posix.cc @@ -39,6 +39,12 @@ void Semaphore::Wait() { bool Semaphore::TimedWait(double seconds) { DCHECK_GE(seconds, 0.0); + + if (std::isinf(seconds)) { + Wait(); + return true; + } + timespec timeout; timeout.tv_sec = seconds; timeout.tv_nsec = (seconds - trunc(seconds)) * 1E9; diff --git a/util/synchronization/semaphore_test.cc b/util/synchronization/semaphore_test.cc index fb4338c0..ccaf742d 100644 --- a/util/synchronization/semaphore_test.cc +++ b/util/synchronization/semaphore_test.cc @@ -43,6 +43,18 @@ TEST(Semaphore, TimedWaitTimeout) { EXPECT_FALSE(semaphore.TimedWait(0.01)); // 10ms } +TEST(Semaphore, TimedWaitInfinite_0) { + Semaphore semaphore(0); + semaphore.Signal(); + EXPECT_TRUE(semaphore.TimedWait(std::numeric_limits::infinity())); +} + +TEST(Semaphore, TimedWaitInfinite_1) { + Semaphore semaphore(1); + EXPECT_TRUE(semaphore.TimedWait(std::numeric_limits::infinity())); + semaphore.Signal(); +} + struct ThreadMainInfo { #if defined(OS_POSIX) pthread_t pthread; diff --git a/util/synchronization/semaphore_win.cc b/util/synchronization/semaphore_win.cc index 962c7bae..2f26c237 100644 --- a/util/synchronization/semaphore_win.cc +++ b/util/synchronization/semaphore_win.cc @@ -14,6 +14,7 @@ #include "util/synchronization/semaphore.h" +#include #include #include "base/logging.h" @@ -38,6 +39,12 @@ void Semaphore::Wait() { bool Semaphore::TimedWait(double seconds) { DCHECK_GE(seconds, 0.0); + + if (std::isinf(seconds)) { + Wait(); + return true; + } + DWORD rv = WaitForSingleObject(semaphore_, static_cast(seconds * 1E3)); PCHECK(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT) << "WaitForSingleObject"; return rv == WAIT_OBJECT_0; diff --git a/util/thread/worker_thread.cc b/util/thread/worker_thread.cc index 595ef8f4..3cf48970 100644 --- a/util/thread/worker_thread.cc +++ b/util/thread/worker_thread.cc @@ -15,7 +15,6 @@ #include "util/thread/worker_thread.h" #include "base/logging.h" -#include "util/synchronization/semaphore.h" #include "util/thread/thread.h" namespace crashpad { diff --git a/util/thread/worker_thread.h b/util/thread/worker_thread.h index 321d9184..97fb6ecd 100644 --- a/util/thread/worker_thread.h +++ b/util/thread/worker_thread.h @@ -18,6 +18,7 @@ #include #include "base/macros.h" +#include "util/synchronization/semaphore.h" namespace crashpad { @@ -40,11 +41,16 @@ class WorkerThread { virtual ~Delegate() {} }; + //! \brief A delay or interval argument that causes an indefinite wait. + static constexpr double kIndefiniteWait = Semaphore::kIndefiniteWait; + //! \brief Creates a new WorkerThread that is not yet running. //! //! \param[in] work_interval The time interval in seconds at which the \a //! delegate runs. The interval counts from the completion of - //! Delegate::DoWork() to the next invocation. + //! Delegate::DoWork() to the next invocation. This can be + //! #kIndefiniteWait if work should only be done when DoWorkNow() is + //! called. //! \param[in] delegate The work delegate to invoke every interval. WorkerThread(double work_interval, Delegate* delegate); ~WorkerThread(); @@ -55,7 +61,8 @@ class WorkerThread { //! //! \param[in] initial_work_delay The amount of time in seconds to wait //! before invoking the \a delegate for the first time. Pass `0` for - //! no delay. + //! no delay. This can be #kIndefiniteWait if work should not be done + //! until DoWorkNow() is called. void Start(double initial_work_delay); //! \brief Stops the worker thread from running. diff --git a/util/util.gyp b/util/util.gyp index 7ef7a0ca..5bc4de6f 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -168,6 +168,7 @@ 'stdlib/strlcpy.h', 'stdlib/strnlen.cc', 'stdlib/strnlen.h', + 'stdlib/thread_safe_vector.h', 'string/split_string.cc', 'string/split_string.h', 'synchronization/semaphore_mac.cc', diff --git a/util/util_test.gyp b/util/util_test.gyp index 31323c0b..95dac082 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -87,6 +87,7 @@ 'stdlib/string_number_conversion_test.cc', 'stdlib/strlcpy_test.cc', 'stdlib/strnlen_test.cc', + 'stdlib/thread_safe_vector_test.cc', 'string/split_string_test.cc', 'synchronization/semaphore_test.cc', 'thread/thread_log_messages_test.cc',