From 50cc9dfe39377c498d302eb137266b4e6b2251ec Mon Sep 17 00:00:00 2001 From: Jay Dolan Date: Mon, 12 Oct 2026 15:02:42 -0400 Subject: [PATCH] Add Thread::isCancelled and Operation::isCancelled Both flags are set from another thread while a function polls them, so 2.3.0 asked callers to read them with __atomic_load_n. That put an atomics builtin at every call site. The flags are now @private, and each class has an isCancelled method that loads its flag atomically. The interfaces change, so the version is now 2.4.0. Co-Authored-By: Claude Opus 5.5 (1M context) --- Sources/Objectively/Operation.c | 15 ++++++++++++--- Sources/Objectively/Operation.h | 15 +++++++++++++-- Sources/Objectively/OperationQueue.c | 2 +- Sources/Objectively/Thread.c | 9 +++++++++ Sources/Objectively/Thread.h | 13 +++++++++++-- Sources/Objectively/ThreadPool.c | 2 +- Sources/Objectively/URLSession.c | 12 ++++++------ Tests/Objectively/Thread.c | 16 ++++++++++++++++ Tests/Objectively/ThreadPool.c | 4 ++-- configure.ac | 2 +- 10 files changed, 72 insertions(+), 18 deletions(-) diff --git a/Sources/Objectively/Operation.c b/Sources/Objectively/Operation.c index 402fc5b..646b1ef 100644 --- a/Sources/Objectively/Operation.c +++ b/Sources/Objectively/Operation.c @@ -72,7 +72,7 @@ static void addDependency(Operation *self, Operation *dependency) { */ static void cancel(Operation *self) { - if (__atomic_load_n(&self->isCancelled, __ATOMIC_ACQUIRE) || self->isFinished) { + if ($(self, isCancelled) || self->isFinished) { return; } @@ -133,6 +133,14 @@ static Operation *initWithFunction(Operation *self, OperationFunction function, return self; } +/** + * @fn bool Operation::isCancelled(const Operation *self) + * @memberof Operation + */ +static bool isCancelled(const Operation *self) { + return __atomic_load_n(&self->isCancelled, __ATOMIC_ACQUIRE); +} + /** * @fn bool Operation::isReady(const Operation *self) * @memberof Operation @@ -143,7 +151,7 @@ static bool isReady(const Operation *self) { return false; } - if (__atomic_load_n(&self->isCancelled, __ATOMIC_ACQUIRE)) { + if ($(self, isCancelled)) { return true; } @@ -180,7 +188,7 @@ static void start(Operation *self) { return; } - if (__atomic_load_n(&self->isCancelled, __ATOMIC_ACQUIRE) == false) { + if ($(self, isCancelled) == false) { self->isExecuting = true; self->function(self); self->isExecuting = false; @@ -221,6 +229,7 @@ static void initialize(Class *clazz) { ((OperationInterface *) clazz->interface)->dependencies = dependencies; ((OperationInterface *) clazz->interface)->init = init; ((OperationInterface *) clazz->interface)->initWithFunction = initWithFunction; + ((OperationInterface *) clazz->interface)->isCancelled = isCancelled; ((OperationInterface *) clazz->interface)->isReady = isReady; ((OperationInterface *) clazz->interface)->removeDependency = removeDepdenency; ((OperationInterface *) clazz->interface)->start = start; diff --git a/Sources/Objectively/Operation.h b/Sources/Objectively/Operation.h index 66224d6..8a91776 100644 --- a/Sources/Objectively/Operation.h +++ b/Sources/Objectively/Operation.h @@ -93,8 +93,9 @@ struct Operation { /** * @brief `true` when this Operation has been cancelled, `false` otherwise. - * @remarks Operation::cancel MAY set this from another thread while the Operation executes, so - * the function SHOULD read it with `__atomic_load_n(&operation->isCancelled, __ATOMIC_ACQUIRE)`. + * @remarks Operation::cancel MAY set this from another thread while the Operation executes. + * Read it with Operation::isCancelled. + * @private */ bool isCancelled; @@ -182,6 +183,16 @@ struct OperationInterface { */ Operation *(*initWithFunction)(Operation *self, OperationFunction function, ident data); + /** + * @fn bool Operation::isCancelled(const Operation *self) + * @param self The Operation. + * @return `true` if this Operation has been cancelled, `false` otherwise. + * @remarks This MAY be called from any thread, including from the Operation's function while + * another thread cancels it. + * @memberof Operation + */ + bool (*isCancelled)(const Operation *self); + /** * @fn bool Operation::isReady(const Operation *self) * @param self The Operation. diff --git a/Sources/Objectively/OperationQueue.c b/Sources/Objectively/OperationQueue.c index 6576d98..eaeb10c 100644 --- a/Sources/Objectively/OperationQueue.c +++ b/Sources/Objectively/OperationQueue.c @@ -65,7 +65,7 @@ static void dealloc(Object *self) { static void addOperation(OperationQueue *self, Operation *operation) { assert(operation); - assert(operation->isCancelled == false); + assert($(operation, isCancelled) == false); assert(operation->isExecuting == false); assert(operation->isFinished == false); diff --git a/Sources/Objectively/Thread.c b/Sources/Objectively/Thread.c index 34bd7fb..579496c 100644 --- a/Sources/Objectively/Thread.c +++ b/Sources/Objectively/Thread.c @@ -124,6 +124,14 @@ static Thread *initWithFunction(Thread *self, ThreadFunction function, ident dat return self; } +/** + * @fn bool Thread::isCancelled(const Thread *self) + * @memberof Thread + */ +static bool isCancelled(const Thread *self) { + return __atomic_load_n(&self->isCancelled, __ATOMIC_ACQUIRE); +} + /** * @fn void Thread::join(Thread *self, ident *status) * @memberof Thread @@ -194,6 +202,7 @@ static void initialize(Class *clazz) { ((ThreadInterface *) clazz->interface)->detach = detach; ((ThreadInterface *) clazz->interface)->init = init; ((ThreadInterface *) clazz->interface)->initWithFunction = initWithFunction; + ((ThreadInterface *) clazz->interface)->isCancelled = isCancelled; ((ThreadInterface *) clazz->interface)->join = join; ((ThreadInterface *) clazz->interface)->kill = _kill; ((ThreadInterface *) clazz->interface)->start = start; diff --git a/Sources/Objectively/Thread.h b/Sources/Objectively/Thread.h index 5ee491c..61c0e68 100644 --- a/Sources/Objectively/Thread.h +++ b/Sources/Objectively/Thread.h @@ -75,8 +75,8 @@ struct Thread { /** * @brief `true` when this Thread has been cancelled, `false` otherwise. - * @remarks Thread::cancel sets this from another thread, so the ThreadFunction SHOULD read it - * with `__atomic_load_n(&thread->isCancelled, __ATOMIC_ACQUIRE)`. + * @remarks Thread::cancel sets this from another thread. Read it with Thread::isCancelled. + * @private */ bool isCancelled; @@ -157,6 +157,15 @@ struct ThreadInterface { */ Thread *(*initWithFunction)(Thread *self, ThreadFunction function, ident data); + /** + * @fn bool Thread::isCancelled(const Thread *self) + * @param self The Thread. + * @return `true` if this Thread has been cancelled, `false` otherwise. + * @remarks This MAY be called from any thread, including while another thread cancels this one. + * @memberof Thread + */ + bool (*isCancelled)(const Thread *self); + /** * @fn void Thread::join(Thread *self, ident *status) * @brief Wait for the specified Thread to terminate. diff --git a/Sources/Objectively/ThreadPool.c b/Sources/Objectively/ThreadPool.c index b76b736..719e359 100644 --- a/Sources/Objectively/ThreadPool.c +++ b/Sources/Objectively/ThreadPool.c @@ -250,7 +250,7 @@ static ident run(Thread *thread) { while (true) { - while (self->head == NULL && thread->isCancelled == false) { + while (self->head == NULL && $(thread, isCancelled) == false) { $(condition, wait); } diff --git a/Sources/Objectively/URLSession.c b/Sources/Objectively/URLSession.c index f4e4c17..09c635d 100644 --- a/Sources/Objectively/URLSession.c +++ b/Sources/Objectively/URLSession.c @@ -63,7 +63,7 @@ static ident taskWithRequest(URLSession *self, ident task, URLRequest *request, bool isInvalidated; synchronized(self->condition, { - isInvalidated = self->thread->isCancelled; + isInvalidated = $(self->thread, isCancelled); if (isInvalidated == false) { $(self->tasks, addObject, task); $(self->condition, signal); @@ -91,7 +91,7 @@ static URLSessionDataTask *dataTaskWithRequest(URLSession *self, URLRequest *req bool isInvalidated; synchronized(self->condition, { - isInvalidated = self->thread->isCancelled; + isInvalidated = $(self->thread, isCancelled); }); if (isInvalidated) { @@ -107,7 +107,7 @@ static URLSessionDataTask *dataTaskWithRequest(URLSession *self, URLRequest *req } synchronized(self->condition, { - isInvalidated = self->thread->isCancelled; + isInvalidated = $(self->thread, isCancelled); if (isInvalidated == false) { $(self->tasks, addObject, task); $(self->condition, signal); @@ -212,10 +212,10 @@ static ident run(Thread *thread) { bool idle; synchronized(self->condition, { - while (!hasWork(self) && !(thread->isCancelled && self->tasks->count == 0)) { + while (!hasWork(self) && !($(thread, isCancelled) && self->tasks->count == 0)) { $(self->condition, wait); } - idle = thread->isCancelled && self->tasks->count == 0; + idle = $(thread, isCancelled) && self->tasks->count == 0; }); if (idle) { @@ -392,7 +392,7 @@ static void invalidateAndCancel(URLSession *self) { bool isInvalidated; synchronized(self->condition, { - isInvalidated = self->thread->isCancelled; + isInvalidated = $(self->thread, isCancelled); if (isInvalidated == false) { $(self->thread, cancel); $(self->condition, signal); diff --git a/Tests/Objectively/Thread.c b/Tests/Objectively/Thread.c index b949b8e..5937115 100644 --- a/Tests/Objectively/Thread.c +++ b/Tests/Objectively/Thread.c @@ -127,11 +127,27 @@ START_TEST(cond) { } END_TEST +START_TEST(isCancelled) { + + Thread *thread = $(alloc(Thread), initWithFunction, increment, NULL); + ck_assert(thread != NULL); + + ck_assert($(thread, isCancelled) == false); + + $(thread, cancel); + + ck_assert($(thread, isCancelled)); + + release(thread); + +} END_TEST + int main(int argc, char **argv) { TCase *tcase = tcase_create("Thread"); tcase_add_test(tcase, thread); tcase_add_test(tcase, cond); + tcase_add_test(tcase, isCancelled); Suite *suite = suite_create("Thread"); suite_add_tcase(suite, tcase); diff --git a/Tests/Objectively/ThreadPool.c b/Tests/Objectively/ThreadPool.c index 28d1721..2967d99 100644 --- a/Tests/Objectively/ThreadPool.c +++ b/Tests/Objectively/ThreadPool.c @@ -195,7 +195,7 @@ static ident waitForGateOrCancel(Thread *thread) { $(condition, broadcast); }); - while (__atomic_load_n(&thread->isCancelled, __ATOMIC_ACQUIRE) == false) { + while ($(thread, isCancelled) == false) { usleep(1000); } @@ -281,7 +281,7 @@ static ident untilCancelled(Thread *thread) { $(condition, broadcast); }); - while (__atomic_load_n(&thread->isCancelled, __ATOMIC_ACQUIRE) == false) { + while ($(thread, isCancelled) == false) { usleep(1000); } diff --git a/configure.ac b/configure.ac index f2de154..1ed1b02 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ AC_PREREQ(2.69) -AC_INIT([Objectively], [2.3.0], [jay@jaydolan.com]) +AC_INIT([Objectively], [2.4.0], [jay@jaydolan.com]) AC_CONFIG_HEADERS([Sources/Objectively/Config.h]) AC_CONFIG_MACRO_DIR([m4])