Contributor note: I am a fledgling developer, and I tried to make this change carefully. I reproduced the bug, narrowed the claim, wrote a direct regression test, ran the focused tests and Ruff, and had the final diff independently reviewed with Opus 5 before asking upstream to look at it. That does not replace maintainer review or CI; it is the process I used to reduce the odds of wasting their time.
A long-lived Hermes process ran out of file descriptors while holding repeated SQLite database and WAL handles. The confirmed bug was smaller than the whole incident: SessionDB could track reader connections created by worker threads, but its owner thread could not close them during shutdown.
The missing detail was one SQLite connection option: check_same_thread=False.
The reader stays thread-local during normal use. Only the shutdown drain crosses threads.
What the PR changes
- Open per-thread WAL readers with
check_same_thread=False. - Warn if a thread-affinity
ProgrammingErrorstill blocks shutdown. - Test the lifecycle directly: worker creates reader, worker exits, owner closes, retained reader reports
closed database.
Why the test got smaller
The first version counted process file descriptors and coordinated several worker threads. That tested the symptom indirectly and added platform noise. The final test is simpler and stronger: it keeps one worker-created connection, lets the worker finish, calls SessionDB.close() from the owner, and proves the connection is actually closed.
| Question | Answer |
|---|---|
| Does the test catch the old behavior? | Yes. Current upstream produces the original same-thread error instead of closed database. |
| Does this prove every FD leak is gone? | No. It fixes one confirmed SessionDB reader lifecycle class. |
| What did upstream automation say? | Hermes Sweeper recommended keep_open, rated salvageability high, and said the one-line connection-policy change and lifecycle test align with the bug. |
| Has upstream CI approved it? | Not yet. GitHub currently reports no checks for the fork head. |
The important boundary: disabling SQLite's Python thread-affinity check is not a promise that arbitrary concurrent operations are safe. This patch covers cleanup after the reader's worker has finished.
Read Hermes PR #74304 →Small bug, small patch, direct test. The hard part was making the claim no larger than the evidence.