diff --git a/internal/platform/BUILD b/internal/platform/BUILD index 2100f39d..a26db59b 100644 --- a/internal/platform/BUILD +++ b/internal/platform/BUILD @@ -575,6 +575,7 @@ cc_test( "//internal/flags:nearby_flags", "//internal/platform/flags:platform_flags", "//internal/platform/implementation:comm", + "//internal/platform/implementation:types", "//internal/proto:credential_cc_proto", "//internal/test", "//proto:connections_enums_cc_proto", diff --git a/internal/platform/cancelable_alarm.h b/internal/platform/cancelable_alarm.h index e90d502c..deb986bc 100644 --- a/internal/platform/cancelable_alarm.h +++ b/internal/platform/cancelable_alarm.h @@ -15,15 +15,16 @@ #ifndef PLATFORM_PUBLIC_CANCELABLE_ALARM_H_ #define PLATFORM_PUBLIC_CANCELABLE_ALARM_H_ -#include -#include #include #include +#include "absl/base/thread_annotations.h" #include "absl/functional/any_invocable.h" +#include "absl/strings/string_view.h" #include "internal/platform/cancelable.h" #include "internal/platform/mutex.h" #include "internal/platform/mutex_lock.h" +#include "absl/time/time.h" #include "internal/platform/scheduled_executor.h" namespace nearby { @@ -49,18 +50,18 @@ class CancelableAlarm { } ~CancelableAlarm() = default; - bool Cancel() { + bool Cancel() ABSL_LOCKS_EXCLUDED(mutex_) { MutexLock lock(&mutex_); return cancelable_.Cancel(); } - bool IsValid() { + bool IsValid() ABSL_LOCKS_EXCLUDED(mutex_) { MutexLock lock(&mutex_); return cancelable_.IsValid(); } private: - void Schedule() { + void Schedule() ABSL_LOCKS_EXCLUDED(mutex_) { MutexLock lock(&mutex_); cancelable_ = scheduled_executor_->Schedule( [this]() { @@ -72,7 +73,7 @@ class CancelableAlarm { Mutex mutex_; std::string name_; - Cancelable cancelable_; + Cancelable cancelable_ ABSL_GUARDED_BY(mutex_); ScheduledExecutor* scheduled_executor_; absl::Duration delay_; absl::AnyInvocable runnable_; diff --git a/internal/platform/cancelable_alarm_test.cc b/internal/platform/cancelable_alarm_test.cc index be9d67ef..0c04bace 100644 --- a/internal/platform/cancelable_alarm_test.cc +++ b/internal/platform/cancelable_alarm_test.cc @@ -14,12 +14,11 @@ #include "internal/platform/cancelable_alarm.h" -#include - #include "gtest/gtest.h" #include "absl/time/time.h" #include "internal/platform/atomic_boolean.h" #include "internal/platform/atomic_reference.h" +#include "internal/platform/implementation/system_clock.h" #include "internal/platform/scheduled_executor.h" namespace nearby { diff --git a/internal/platform/implementation/apple/scheduled_executor.mm b/internal/platform/implementation/apple/scheduled_executor.mm index b4a5e2ec..e7700c3c 100644 --- a/internal/platform/implementation/apple/scheduled_executor.mm +++ b/internal/platform/implementation/apple/scheduled_executor.mm @@ -16,32 +16,69 @@ #import +#include #include #include #include "absl/time/time.h" -#import "internal/platform/implementation/apple/atomic_boolean.h" #include "internal/platform/runnable.h" -// This wraps the C++ Runnable in an Obj-C object for memory management. It is retained by the -// dispatch block below, and deleted when the block is released. -@interface GNCRunnableWrapper : NSObject { +// Defines the state of a scheduled task. This enum is at global scope +// to be accessible by the global Objective-C++ classes below. +enum class GNCScheduledTaskState { + kScheduled, + kRunning, + kDone, + kCanceled, +}; + +// An Objective-C object to hold the shared state between the cancelable handle +// and the scheduled block. Its lifetime is managed by ARC. +@interface GNCScheduledTask : NSObject { @public + // The user-provided runnable to execute. nearby::Runnable _runnable; - std::unique_ptr _canceled; + // The atomic state machine for the task's lifecycle. + std::atomic _state; +} +- (instancetype)initWithRunnable:(nearby::Runnable &&)runnable; +@end + +@implementation GNCScheduledTask +- (instancetype)initWithRunnable:(nearby::Runnable &&)runnable { + if ((self = [super init])) { + _runnable = std::move(runnable); + _state = GNCScheduledTaskState::kScheduled; + } + return self; } @end -@implementation GNCRunnableWrapper +// An implementation of api::Cancelable that controls a GNCScheduledTask. +// This is an Objective-C++ class to allow for a __strong ivar, which lets ARC +// manage the lifetime of the GNCScheduledTask object. +class ExecutorCancelable : public nearby::api::Cancelable { + public: + explicit ExecutorCancelable(GNCScheduledTask *task) : task_(task) {} + ~ExecutorCancelable() override = default; + ExecutorCancelable(const ExecutorCancelable &) = delete; + ExecutorCancelable &operator=(const ExecutorCancelable &) = delete; -+ (instancetype)wrapperWithRunnable:(nearby::Runnable)runnable { - GNCRunnableWrapper *wrapper = [[GNCRunnableWrapper alloc] init]; - wrapper->_runnable = std::move(runnable); - wrapper->_canceled = std::make_unique(false); - return wrapper; -} + bool Cancel() override { + // If the task has already been canceled, do nothing and return true. + if (task_->_state.load() == GNCScheduledTaskState::kCanceled) { + return true; + } + GNCScheduledTaskState expected = GNCScheduledTaskState::kScheduled; + // Atomically transition from Scheduled to Canceled. This will only + // succeed if the task has not already started running. + return task_->_state.compare_exchange_strong(expected, GNCScheduledTaskState::kCanceled); + } -@end + private: + // ARC-managed strong reference to the shared task state. + __strong GNCScheduledTask *task_; +}; @implementation GNCOperationQueueImpl @@ -59,32 +96,16 @@ namespace apple { static const std::int64_t kExecutorShutdownDefaultTimeout = 500; // 0.5 seconds -// This Cancelable references a Runnable and a cancel method that sets its canceled boolean to true. -class CancelableForRunnable : public api::Cancelable { - public: - explicit CancelableForRunnable(GNCRunnableWrapper *runnable) : runnable_(runnable) {} - CancelableForRunnable() = default; - ~CancelableForRunnable() override = default; - CancelableForRunnable(const CancelableForRunnable &) = delete; - CancelableForRunnable &operator=(const CancelableForRunnable &) = delete; - - // api::Cancelable: - bool Cancel() override { - runnable_->_canceled->Set(true); - return true; - } - - private: - GNCRunnableWrapper *runnable_; -}; - ScheduledExecutor::ScheduledExecutor() { impl_ = [GNCOperationQueueImpl implWithMaxConcurrency:1]; } ScheduledExecutor::ScheduledExecutor(int max_concurrency) { impl_ = [GNCOperationQueueImpl implWithMaxConcurrency:max_concurrency]; } -ScheduledExecutor::~ScheduledExecutor() { impl_ = nil; } +ScheduledExecutor::~ScheduledExecutor() { + Shutdown(); + impl_ = nil; +} void ScheduledExecutor::Shutdown() { Shutdown(kExecutorShutdownDefaultTimeout); } @@ -92,24 +113,35 @@ std::shared_ptr ScheduledExecutor::Schedule(Runnable &&runnable absl::Duration duration) { if (impl_.shuttingDown) return std::shared_ptr(nullptr); - // Wrap the runnable in an Obj-C object so it can be referenced by the delayed block. - GNCRunnableWrapper *wrapper = [GNCRunnableWrapper wrapperWithRunnable:std::move(runnable)]; - CancelableForRunnable *cancelable = new CancelableForRunnable(wrapper); - GNCOperationQueueImpl *impl = impl_; // don't capture |this| + // Create the shared task object. ARC will manage its lifetime. + GNCScheduledTask *task = [[GNCScheduledTask alloc] initWithRunnable:std::move(runnable)]; + + // Create the cancelable handle that the caller will own. + auto cancelable = std::make_shared(task); + + __weak __typeof__(impl_) weakImpl = impl_; dispatch_after( dispatch_time(DISPATCH_TIME_NOW, absl::ToInt64Milliseconds(duration) * NSEC_PER_MSEC), - dispatch_get_global_queue(DISPATCH_TARGET_QUEUE_DEFAULT, 0), ^{ - [impl.queue addOperationWithBlock:^{ - // Execute the runnable only if the executor is not shutting down, and the runnable isn't - // canceled. - // Warning: This block should reference only Obj-C objects, and never C++ objects. - if (!impl.shuttingDown && !wrapper->_canceled->Get()) { - wrapper->_runnable(); + dispatch_get_global_queue(DISPATCH_TARGET_QUEUE_DEFAULT, 0), + // The block captures the task object, and ARC will keep it alive. + [weakImpl, task]() { + __strong __typeof__(weakImpl) strongImpl = weakImpl; + if (!strongImpl || strongImpl.shuttingDown) { + return; + } + [strongImpl.queue addOperationWithBlock:^{ + if (strongImpl.shuttingDown) { + return; + } + GNCScheduledTaskState expected = GNCScheduledTaskState::kScheduled; + if (task->_state.compare_exchange_strong(expected, GNCScheduledTaskState::kRunning)) { + task->_runnable(); + task->_state.store(GNCScheduledTaskState::kDone); } }]; }); - return std::shared_ptr(cancelable); + return cancelable; } void ScheduledExecutor::Execute(Runnable &&runnable) { DoSubmit(std::move(runnable)); }