Add ThreadPool, and make URLSession requests cancellable by jdolan · Pull Request #42 · jdolan/Objectively

jdolan · 2026-10-12T18:09:06Z

Quetoo needs to stop its client game's background work before it unloads the module (jdolan/quetoo#1106). That needs a thread pool it can own and release, and HTTP requests that can be interrupted. This PR adds both, and flattens the locals structs on the way.

Commits

Flatten the locals structs into @private members. Operation, OperationQueue, URLSession and URLSessionTask now declare private members as ordinary members, in alphabetical order after the parent struct and interface[0]. AGENTS.md records the convention.
Add ThreadPool, and run OperationQueue on it.
- A fixed set of Threads takes ThreadFunctions from a FIFO queue of plain ThreadPoolWork records.
- execute returns a handle. wait returns the result and frees the handle, and runs the work on the caller if it is still queued, so nested waits can not deadlock.
- detach queues work with no handle.
- Dealloc sets isCancelled on the Threads, which drain the queue and exit.
- Operation::cancel now also marks a running Operation.
Make URLSession requests cancellable, including synchronous ones.
- The progress callback now runs, and aborts a cancelled transfer, so execute returns within about one second.
- Task state changes are made under the session's condition.
- The run loop no longer spins while only synchronous or suspended tasks exist.
- URLSessionTask::result holds the curl result. RESTClient returns 0 and no data unless the transfer completed.
- The factories return NULL after invalidateAndCancel.

Behaviour changes

Tasks no longer retain their session. The session thread releases tasks, so a task could drop the session's last reference on that thread. The session then aborted, because it tried to join its own thread; the stress tests reproduced this. Deallocating a session now cancels its tasks, and a task MUST NOT be used after its session is deallocated.
Task progress callbacks now fire, and bytesExpectedToReceive is set. Before this change they never ran.
A cancelled or truncated transfer now gives RESTClient status 0. Before, it could report 200 with partial data.
The version is 2.3.0, because instance layouts change. ObjectivelyGPU and ObjectivelyMVC need a rebuild and a >= 2.3.0 requirement.

Testing

make check: 27 of 27 suites pass.
New network-free tests:
- 9 ThreadPool tests;
- 6 cancel tests against a local socket that never responds (sync, async, suspend, RESTClient, and an invalidated session).
Stress runs with no failures:
- ThreadPool and OperationQueue, 800 runs under parallel load;
- the offline URLSession suite, 400 runs under load.
The Xcode library and the Objectively-ThreadPool scheme build. The Windows build is checked only by CI.

Not changed

The session thread still spins while an async transfer is active (curl_multi_wait with a 0 timeout). That existed before this change.
The existing URLSession and RESTClient network tests depend on github.com and httpbin.org. Both rate-limited the test runs during this work.

🤖 Generated with Claude Code

@Private

Operation, OperationQueue, URLSession and URLSessionTask nested their private members in an anonymous `locals` struct. Most types, and all of ObjectivelyMVC, instead declare private members as ordinary members documented with @Private. These four now do the same, with their members in alphabetical order after the parent struct and interface[0]. No code outside Objectively reads these members. AGENTS.md now records the convention. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

ThreadPool runs ThreadFunctions on a fixed set of Threads. Submitted functions wait in a FIFO queue of plain ThreadPoolWork records until a Thread is free: - execute returns a ThreadPoolWork; wait returns the function's result and frees the record. The handle is the job, not the Thread, so a Thread that has gone on to other work can never be waited on. - detach queues a function with no handle. - wait runs a function that is still in the queue on the calling thread, so a function can wait on work of its own pool without a deadlock. - Deallocating a ThreadPool sets isCancelled on its Threads, which run the rest of the queue before they exit. OperationQueue now runs its loops on a ThreadPool of its own, and Operation::cancel also marks an Operation that is running, so that its function can return early. The instance layouts change, so the version is now 2.3.0. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

URLSessionTask::execute never checked the task state, and the progress callback never ran because CURLOPT_NOPROGRESS was never cleared. So a synchronous request could not be interrupted, and cancelling a task inside execute let the session thread tear down a curl handle that curl_easy_perform was still using. - The progress callback now runs, and aborts the transfer when the task is cancelling. A cancelled execute returns within about one second. - execute marks its task while it runs, and the run loop leaves such a task alone. Task state changes are made under the session's condition, so the run loop can not overwrite a cancel. - The run loop waits while no task needs it, rather than spinning while a synchronous request runs. - URLSessionTask::result holds the curl result. RESTClient reports status 0 and no data unless the transfer completed, so a cancelled or truncated transfer no longer looks like a 200. - After invalidateAndCancel, the task factories return NULL. - Tasks no longer retain their session. The session thread releases tasks, so a task could release the session's last reference on that thread, which then aborted joining itself. Deallocating a session now cancels its tasks, and a task MUST NOT be used after its session is deallocated. New tests use a local socket that never responds, so they need no network. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

URLSession and Operation cancellation contain data races, and cached task transitions can leave the session thread asleep indefinitely.

3 open findings

What changed in this PR

Adds an owned thread pool, migrates operation queues to it, and makes URL requests cancellable.

Changes:

Adds ThreadPool with tests and build-system integration.
Adds URLSession cancellation and RESTClient failure handling.
Flattens private instance layouts and bumps ABI version to 2.3.0.

File	Description
`AGENTS.md`	Documents instance-member layout rules.
`Documentation/index.md`	Lists ThreadPool support.
`README.md`	Lists ThreadPool support.
`configure.ac`	Bumps version to 2.3.0.
`Sources/Objectively.h`	Exports ThreadPool.
`Sources/Objectively/Makefile.am`	Builds and installs ThreadPool.
`Sources/Objectively/Operation.c`	Flattens members and expands cancellation.
`Sources/Objectively/Operation.h`	Updates Operation layout and API docs.
`Sources/Objectively/OperationQueue.c`	Runs operations through ThreadPool.
`Sources/Objectively/OperationQueue.h`	Updates queue layout for ThreadPool.
`Sources/Objectively/RESTClient.c`	Handles failed and cancelled transfers.
`Sources/Objectively/RESTClient.h`	Documents failure status semantics.
`Sources/Objectively/ThreadPool.c`	Implements the thread pool.
`Sources/Objectively/ThreadPool.h`	Defines the ThreadPool API.
`Sources/Objectively/URLSession.c`	Coordinates cancellation and task states.
`Sources/Objectively/URLSession.h`	Documents session/task lifetime behavior.
`Sources/Objectively/URLSessionDataTask.c`	Integrates cancellation and caching results.
`Sources/Objectively/URLSessionDownloadTask.c`	Uses flattened task handles.
`Sources/Objectively/URLSessionTask.c`	Implements cancellable transfers.
`Sources/Objectively/URLSessionTask.h`	Expands task state and result data.
`Sources/Objectively/URLSessionUploadTask.c`	Uses flattened task handles.
`Tests/Objectively/.gitignore`	Ignores ThreadPool test output.
`Tests/Objectively/Makefile.am`	Registers ThreadPool tests.
`Tests/Objectively/ThreadPool.c`	Tests pool execution and shutdown.
`Tests/Objectively/URLSession.c`	Tests cancellation and invalidation.
`Objectively.xcodeproj/project.pbxproj`	Registers ThreadPool sources and tests.
`Objectively.xcodeproj/xcshareddata/xcschemes/Objectively-ThreadPool.xcscheme`	Adds the ThreadPool Xcode scheme.
`Objectively.vs15/Objectively.vcxproj`	Registers ThreadPool for Visual Studio.
`Objectively.vs15/Objectively.vcxproj.filters`	Adds Visual Studio filters.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

jdolan · 2026-10-12T18:36:10Z

+      while (!hasWork(self) && !(thread->isCancelled && self->tasks->count == 0)) {
+        $(self->condition, wait);


No change: I believe this one can not happen. A data task only gets a cachedResponse in URLSession::dataTaskWithRequest, and on a cache hit that method returns the task before adding it to self->tasks (URLSession.c, the early return task;). So the cached paths in URLSessionDataTask.c (cancel, execute, resume) only ever run on tasks the run loop never sees, and hasWork never waits on them. Leaving this thread open for a human to confirm.

Operation::cancel now marks a running Operation, and ThreadPool jobs poll Thread::isCancelled, so both flags are written on one thread while another reads them. They are now stored and loaded atomically, and their docs say how a function should read them. The progress callback read the task state without the session lock that every writer holds; it now takes the lock. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The engine's own pool handed out the worker itself as the handle to wait on. A THREAD_NO_WAIT worker went back to the pool when it finished, so a handle to it could later name another job, and waiting on it failed an assert or set a running worker idle. jdolan#1106 was one instance. Objectively's ThreadPool queues work and returns a handle to the job, not the worker, so that can no longer happen: - The engine pool is now a ThreadPool, sized by the threads cvar as before. Cl_UpdateScene, material loads, screenshots and quemap's Work() use it. - The client game no longer shares the engine pool. The client creates a ThreadPool of four threads for each client game, passes it as cgi.threadPool, and releases it in Cl_ShutdownCgame right after the module's Shutdown. That waits for all of its work, so none can run after the module is closed. The hero image fetch also stops between downloads once the pool is released. - cgi.Thread and cgi.Wait are gone, so CGAME_API_VERSION is now 68. - check_thread tested the removed API. Objectively's ThreadPool tests replace it. Requires Objectively 2.3.0 (jdolan/Objectively#42). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

jdolan and others added 4 commits October 12, 2026 14:07

Alpha

a52db4b

Copilot AI balanced review requested due to automatic review settings October 12, 2026 18:09

Copilot started reviewing on behalf of jdolan October 12, 2026 18:09

View session

Copilot AI reviewed Oct 12, 2026

View reviewed changes

jdolan mentioned this pull request Oct 12, 2026

Crash Report jdolan/quetoo#1106

Closed

jdolan merged commit b22ce2d into main Oct 12, 2026
4 checks passed

jdolan mentioned this pull request Oct 12, 2026

Fixes #1106: Absorb Objectively fixes to allow Thread cancellation of HTTP requests and other async work jdolan/quetoo#1112

Merged

Navigation Menu

Repository navigation

Add ThreadPool, and make URLSession requests cancellable - #42

Add ThreadPool, and make URLSession requests cancellable#42
jdolan merged 5 commits into
mainfrom
feature/thread-pool

jdolan commented Oct 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

jdolan Oct 12, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		while (!hasWork(self) && !(thread->isCancelled && self->tasks->count == 0)) {
		$(self->condition, wait);

Conversation

jdolan commented Oct 12, 2026

Commits

Behaviour changes

Testing

Not changed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

jdolan Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants