Repository navigation
Close the connections of exited threads, keep inherited connections out of forked children and make fork wait for in-flight transactions - #13
Merged
Conversation
…ut of forked children and make fork wait for in-flight transactions
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.
Two related problems in the per-thread SQLite connection handling, upstream issues 295, 325 and 336 and the territory of upstream pull request 299.
What was wrong.
Cache.close()closed only the calling thread's connection; connections opened by other threads were closed by garbage collection, which emitsResourceWarning: unclosed database(244 in the suite on 3.14) and in long-lived thread-heavy programs exhausts file descriptors. Acrossfork(), the child inherited the parent's connections and the pid check in_conclosed them, which SQLite forbids: reproduced on this machine with Python's own SQLite, a writer thread plus 60 forks left 28 children hung, and a fork inside another thread's transaction left the child blocked for 60 s and then failing with "database is locked". Upstream pull request 299 fixes the fork case by wrapping every transaction in one process-wide lock, which serializes all transactions in the process, so it was not merged.Change (
diskcache/core.py):check_same_thread=False; each is still used by one thread at a time. Opening a connection closes the stale ones;close()closes the calling thread's connection and those of exited threads and leaves live threads alone, with lazy reopen as before; a finalizer closes everything when a Cache is garbage collected._transact: two lock operations per transaction, no serialization. Before fork, new transactions are held back and fork waits, up to 60 s, for transactions in flight in other threads, then each cache closes the forking thread's idle connection and those of exited threads. After fork in the parent, transactions proceed. After fork in the child, every inherited connection is moved to a module-level list that keeps it referenced for the life of the child, so neither code nor garbage collection ever closes it, which is what SQLite's documentation asks for. The pid check in_conandclose()does the same as a fallback. Forking inside the forking thread's own transaction is unsupported and documented.os.register_at_forkis used only where it exists, so Windows is unaffected.close(), fork behaviour, and a limitation found during the analysis: Apple's systemlibsqlite3segfaults onsqlite3.connect()in any forked child once the parent has used SQLite at all, which no library can fix;multiprocessing's defaultspawnon macOS avoids it..pylintrcraisesmax-module-linesto 4000;core.pyis now 3113 lines.Tests (15 new in
tests/test_core.py): exited-thread connections closed byclose()withoutResourceWarningforthreadingand foreign_threadthreads; live threads' connections untouched; lost thread-local state closes the orphan; a garbage-collected Cache closes its connections; the child handler, the pid fallback,close()andclose_all()in a simulated child discard without closing; the before-fork handler closes idle connections and skips inside a transaction; gate timeout warning, nesting, child reset and blocking of new transactions while forking; two real fork tests (skipped whereos.forkis missing): the issue-325 pattern with the child inheriting no connections, and a fork that must wait for another thread's open transaction, which fails without the gate.Verified locally: 357 tests pass at 99.04 % on Python 3.12 with Django 4.2 and on 3.14 with Django 6.0, the latter with
-W error::ResourceWarning; set, get and transaction timings are within noise of master and eight-thread throughput is unchanged; bluecheck, isortcheck, flake8, mypy, pylint, doc8 and rstcheck pass.