From bda3a779bdd567e5b889ceebffa86cdd8faec725 Mon Sep 17 00:00:00 2001 From: Guogang Li Date: Tue, 4 Apr 2023 11:39:38 -0700 Subject: [PATCH] Fixed the crash in Timer::Stop PiperOrigin-RevId: 521822379 --- .../platform/implementation/windows/BUILD | 2 ++ .../platform/implementation/windows/timer.cc | 9 +++++++- .../platform/implementation/windows/timer.h | 22 ++++++++++++------- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/internal/platform/implementation/windows/BUILD b/internal/platform/implementation/windows/BUILD index a4da868d..2c134c70 100644 --- a/internal/platform/implementation/windows/BUILD +++ b/internal/platform/implementation/windows/BUILD @@ -50,6 +50,8 @@ cc_library( "//internal/platform:logging", "//internal/platform/implementation:types", "//internal/platform/implementation/windows/generated:types", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/synchronization", ], ) diff --git a/internal/platform/implementation/windows/timer.cc b/internal/platform/implementation/windows/timer.cc index c960bf04..b6fbed51 100644 --- a/internal/platform/implementation/windows/timer.cc +++ b/internal/platform/implementation/windows/timer.cc @@ -16,6 +16,7 @@ #include +#include "absl/synchronization/mutex.h" #include "internal/platform/logging.h" namespace nearby { @@ -25,6 +26,8 @@ Timer::~Timer() { Stop(); } bool Timer::Create(int delay, int interval, absl::AnyInvocable callback) { + absl::MutexLock lock(&mutex_); + if ((delay < 0) || (interval < 0)) { NEARBY_LOGS(WARNING) << "Delay and interval shouldn\'t be negative value."; return false; @@ -58,6 +61,8 @@ bool Timer::Create(int delay, int interval, } bool Timer::Stop() { + absl::MutexLock lock(&mutex_); + if (timer_queue_handle_ == nullptr) { return true; } @@ -81,6 +86,8 @@ bool Timer::Stop() { } bool Timer::FireNow() { + absl::MutexLock lock(&mutex_); + if (!timer_queue_handle_ || !callback_) { return false; } @@ -92,7 +99,7 @@ bool Timer::FireNow() { void CALLBACK Timer::TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired) { absl::AnyInvocable* callback = reinterpret_cast*>(lpParam); - if (*callback != NULL) { + if (*callback != nullptr) { (*callback)(); } } diff --git a/internal/platform/implementation/windows/timer.h b/internal/platform/implementation/windows/timer.h index 2c9edd49..9c56c253 100644 --- a/internal/platform/implementation/windows/timer.h +++ b/internal/platform/implementation/windows/timer.h @@ -17,6 +17,10 @@ #include +#include + +#include "absl/base/thread_annotations.h" +#include "absl/synchronization/mutex.h" #include "internal/platform/implementation/timer.h" namespace nearby { @@ -28,18 +32,20 @@ class Timer : public api::Timer { ~Timer() override; bool Create(int delay, int interval, - absl::AnyInvocable callback) override; - bool Stop() override; - bool FireNow() override; + absl::AnyInvocable callback) override + ABSL_LOCKS_EXCLUDED(mutex_); + bool Stop() override ABSL_LOCKS_EXCLUDED(mutex_); + bool FireNow() override ABSL_LOCKS_EXCLUDED(mutex_); private: static void CALLBACK TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired); - int delay_; - int interval_; - absl::AnyInvocable callback_; - HANDLE handle_ = NULL; - HANDLE timer_queue_handle_ = NULL; + mutable absl::Mutex mutex_; + int delay_ ABSL_GUARDED_BY(mutex_); + int interval_ ABSL_GUARDED_BY(mutex_); + absl::AnyInvocable callback_ ABSL_GUARDED_BY(mutex_); + HANDLE handle_ ABSL_GUARDED_BY(mutex_) = nullptr; + HANDLE timer_queue_handle_ ABSL_GUARDED_BY(mutex_) = nullptr; }; } // namespace windows