diff --git a/.github/workflows/threading-benchmark.yml b/.github/workflows/threading-benchmark.yml new file mode 100644 index 00000000..ec44306a --- /dev/null +++ b/.github/workflows/threading-benchmark.yml @@ -0,0 +1,52 @@ +name: Python SDK threading checks + +on: + pull_request: + types: + - opened + - reopened + - synchronize + - labeled + +permissions: + contents: read + +jobs: + threading-benchmark: + name: Python SDK threading checks + runs-on: ubuntu-24.04-arm + # Backstop: In case there is a hang. + timeout-minutes: 20 + if: | + contains(github.event.pull_request.labels.*.name, 'check-threading-benchmark') && + ( + github.event.pull_request.author_association == 'COLLABORATOR' || + github.event.pull_request.author_association == 'MEMBER' || + github.event.pull_request.author_association == 'OWNER' + ) + steps: + - uses: actions/checkout@v4 + + - name: Build perf image + run: make perf-image + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # Make sure crashes would be reported. + - name: Check the harness can detect failures + run: make threading-bench-self-test + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Run thread-safety invariants + run: make threading-bench + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Upload failure logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: threading-invariant-logs + path: tests/perf/reports/*-threads.log + if-no-files-found: ignore diff --git a/.gitignore b/.gitignore index 147e8357..4c4140d5 100644 --- a/.gitignore +++ b/.gitignore @@ -126,3 +126,6 @@ src/c2pa/libs/ # Memory profiling reports tests/perf/reports/*.html tests/perf/reports/*.bin + +# Threading failure logs +tests/perf/reports/*.log diff --git a/Makefile b/Makefile index d4e7fffd..e53e1f2e 100644 --- a/Makefile +++ b/Makefile @@ -172,3 +172,25 @@ memory-use-bench: clean-memory-perf-reports: rm -f tests/perf/reports/*.html tests/perf/reports/*.bin @echo "Cleared tests/perf/reports/" + +# Thread-safety invariants (runs in Docker, same image as the memory benchmark) +# More details for usage are in tests/perf/README.md +THREAD_ROUNDS ?= 20 + +# Checks that the harness itself reports crashes, hangs and plain exceptions +# correctly. A harness that cannot see a failure is indistinguishable from a +# passing run, so this gates the real suite rather than documenting it. +.PHONY: threading-bench-self-test +threading-bench-self-test: perf-image + docker run --rm -v $(PWD):/workspace -e PYTHONPATH=/workspace/src -e GITHUB_TOKEN c2pa-memray-$(PERF_ENV) python -m tests.perf.run_thread_profile --self-test + +# Runs the thread-safety invariant scenarios. Pre-requisite: Docker image built +# using `make perf-image` (or `perf-image-rebuild`). +.PHONY: threading-bench +threading-bench: threading-bench-self-test + docker run --rm -v $(PWD):/workspace $(GH_SUMMARY_MOUNT) -e PYTHONPATH=/workspace/src -e PERF_ENV=$(PERF_ENV) -e THREAD_ROUNDS=$(THREAD_ROUNDS) -e THREAD_HANG_TIMEOUT -e GITHUB_TOKEN -e GITHUB_STEP_SUMMARY c2pa-memray-$(PERF_ENV) python -m tests.perf.run_thread_profile $(SCENARIO_ARG) $(PERF_ARGS) + +.PHONY: clean-threading-reports +clean-threading-reports: + rm -f tests/perf/reports/*-threads.log + @echo "Cleared tests/perf/reports/*-threads.log" diff --git a/demo/10-native-section.html b/demo/10-native-section.html new file mode 100644 index 00000000..0be5b7b4 --- /dev/null +++ b/demo/10-native-section.html @@ -0,0 +1,223 @@ + + +
+ + +A marked stretch of time on one thread, between a call returning and its error being read. Any free inside it destroys the message.
+ +A Python object lives on the process heap: one region of memory shared by every thread. A thread is not a container that holds objects; it is a separate execution position, with its own call stack, walking through that same shared memory.
+ +So nothing switches threads and nothing is handed over. In the probe behind this figure, a Reader created on MainThread and used from worker-1 stayed at the same address the whole time, with the same id(). Two threads simply looked at the same place.
What is shared is the name. An ordinary closure is enough:
+ +r = Reader("image/jpeg", io.BytesIO(img))
+
+def worker():
+ return r.json() # closure captures r
+
+threading.Thread(target=worker).start()
+
+No serialisation, no copy, no transfer step. Compare multiprocessing, where a separate heap per process forces objects to be pickled across: there id(r) would differ and mutations would not be visible. Threads have no such boundary between them.
The glossary sentence that follows is about executing bytecode. The GIL keeps MainThread and worker-1 from running Python instructions in the same instant. Both can still hold a reference to one Reader at once, and one can call close() while the other is mid-call.
Objects do not belong to threads. If they did, a close() on thread B could not reach thread A's handle, and most of this branch would be unnecessary.
Why the window exists. A C interface cannot raise an exception, so failure arrives in two pieces: a return value, and a message fetched by a separate c2pa_error() call. Between the two, arbitrary Python runs.
What the GIL promises. Picture one token. Whichever thread holds it runs Python code; every other thread waits its turn. That is the entire guarantee: which thread's Python statements run right now, nothing about which objects exist or when they get freed.
+ +Why the token changes hands. A native call such as c2pa_reader_json() runs compiled library code, not Python bytecode. The interpreter hands the token to another thread for the whole call, and takes it back when the call returns. That thread can now run any Python it likes, including code that frees objects, while the first thread's native call is still going.
What freeing means here. A Reader or a Context wraps one native handle: a raw pointer the C library allocated. Freeing calls c2pa_free() on it, exactly once. A second free, or a read after one, is undefined behavior on the native side: a memory fault, or silent corruption, with no Python exception.
The cross-wire. Thread A calls a method, hands the token away, and sits inside the native call holding the pointer. Thread B holds the same Python reference, because objects do not belong to threads. With the token free, thread B calls close() or drops the last reference, and either path ends in c2pa_free() on the pointer thread A is still using. The GIL only ever promised A and B would not run Python bytecode at the same instant. A was inside native code at that instant, token already gone.
The native section. A critical section is a lock: whoever wants in waits for the holder to leave, and every other thread is excluded. The native section runs thread B's close() immediately, every time. _in_native_section() checks thread A's own per-thread depth; if it is above zero, close() records the resource on thread A's pending list. Thread A frees it when its own section closes.
Thread-local because the native slot is: one thread's section must not gate another's frees. Depth-counted because native calls nest, and _read_native_error is itself one. A boolean would be cleared by the innermost exit while an outer classification was still reading.
The drain surfaces your exception unchanged. It keeps the first cleanup failure and logs it; the bare raise re-raises whatever the body threw. A cleanup problem is logged, and the error you were reporting is what you get back.
A close() frees a handle another thread has already passed into a native call.
Both threads reach the same object because both hold a reference to it. The GIL keeps them from running Python at the same instant but is handed away entirely during a native call: the GIL figure on page 10 shows what it does and does not cover.
+Locking across the call deadlocks. These native calls run caller-supplied stream callbacks, which can call back into this API, possibly from a new thread. A lock held across the call would deadlock against that re-entry, so the lock here is held only long enough to change a counter.
+Two independent reasons defer a teardown: this object's own call being in flight, and the native section. The recorded flag merges with and, so a "close without freeing" can never be upgraded to a free by a later caller who does not know the pointer already moved.
What gets freed is a function pointer Python created, which the library calls through while signing.
+ +Both threads reach the same object because both hold a reference to it. The GIL keeps them from running Python at the same instant but is handed away entirely during a native call: the GIL figure on page 10 shows what it does and does not cover.
+What a trampoline is. To let native code call a Python function, ctypes builds a small object native can call like a C function. It is an ordinary Python object with ordinary reference counting, and nothing on the native side holds a reference to it. Keeping it alive as long as native might call it is the caller's job; here the Context holds that reference.
The failure with no error. If the callback is already gone when the sign begins, native signs without calling it and reports success. The output file looks signed; the signer never ran. The guard's validity check turns that into an exception.
+The guard is duck-typed, so a caller-supplied context implementing only the published contract still works, at the cost of no in-flight protection. A test exists to ensure the built-in Context never falls into that unprotected branch.
Helper pages for review.
+ +Why? Fixes native memory corruption (which did not raise exception, but SIGSEV/SIGABORT). The visible symptom is a crash somewhere unrelated, a hang, or wrong output.
+ +A critical section protects data from other threads. This protects a stretch of time on one thread: the gap between a call returning and its error being read. A finalizer for an unrelated object, running in that gap, destroys the message.
+ + + +A close() on one thread frees a handle another thread has already passed into a native call. Holding a lock across the call deadlocks, because those calls run your own code.
What gets freed is a function pointer Python created, which native calls through while signing. On main, the file gets signed without the signer ever running, and the call reports success.
Code references are to src/c2pa/c2pa.py; the “before” quotes are from git show main:src/c2pa/c2pa.py.