Repository navigation
Add Thread::isCancelled and Operation::isCancelled - #43
Merged
Merged
Conversation
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) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new Operation accessor lacks interface-level test coverage, and an existing test still accesses its private field directly.
1 open finding
What changed in this PR
Adds thread-safe cancellation accessors for Thread and Operation, replacing direct cancellation-flag reads.
Changes:
- Adds atomic
isCancelledmethods and marks backing fields private. - Migrates internal callers and adds a
Threadtest. - Bumps the ABI version to 2.4.0.
| File | Description |
|---|---|
Tests/Objectively/ThreadPool.c |
Uses the new Thread accessor. |
Tests/Objectively/Thread.c |
Tests Thread cancellation state. |
Sources/Objectively/URLSession.c |
Migrates cancellation checks. |
Sources/Objectively/ThreadPool.c |
Migrates worker cancellation polling. |
Sources/Objectively/Thread.h |
Declares the cancellation accessor. |
Sources/Objectively/Thread.c |
Implements atomic cancellation reads. |
Sources/Objectively/OperationQueue.c |
Uses the Operation accessor. |
Sources/Objectively/Operation.h |
Declares the cancellation accessor. |
Sources/Objectively/Operation.c |
Implements atomic cancellation reads. |
configure.ac |
Bumps the version to 2.4.0. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+140
to
+142
| static bool isCancelled(const Operation *self) { | ||
| return __atomic_load_n(&self->isCancelled, __ATOMIC_ACQUIRE); | ||
| } |
jdolan
added a commit
to jdolan/quetoo
that referenced
this pull request
Oct 12, 2026
Objectively 2.4.0 makes Thread::isCancelled a method that loads the flag atomically, so the client game's jobs no longer need the atomics builtin. Requires Objectively 2.4.0 (jdolan/Objectively#43). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thread::isCancelledandOperation::isCancelledare set from another thread while a function polls them. 2.3.0 asked callers to read them with__atomic_load_n(&thread->isCancelled, __ATOMIC_ACQUIRE), which put an atomics builtin at every call site.@private.Thread::isCancelled(const Thread *)andOperation::isCancelled(const Operation *)load the flag atomically (acquire, paired with the release store incancel).ThreadPool,OperationQueue,URLSession) and the tests use the methods.Threadtest for the method.The interfaces change, so this bumps the version to 2.4.0. ObjectivelyGPU and ObjectivelyMVC need a rebuild and a
>= 2.4.0requirement. Quetoo's three call sites change to$(thread, isCancelled).Testing
make check: 27 of 27 suites pass.ThreadPool,OperationQueueandThread: 600 runs under parallel load, no failures.🤖 Generated with Claude Code