Open each event database once when callers race #327

Open
clawbot wants to merge 1 commits from issue-291-getdb-single-open into next
Collaborator

Fixes #291.

GetDB opened a webhook's event database on a cache miss and only then tried to cache it, so racing callers could each open the file, and the losers closed their copies.

  • One mutex in WebhookDBManager now covers opening. GetDB looks in the cache again under it, so a caller that lost the race waits for the first caller's handle; no second handle is opened, closed or leaked. Reading an already cached database takes no lock.
  • DeleteDB holds the mutex until the files are removed, so GetDB cannot reopen the file in between. CloseAll holds it so an open under way is closed with the rest, not cached afterwards.
  • Opens of different webhooks now wait for each other; each webhook is opened once per process.
  • The new test starts many callers on one webhook at once and checks they share one handle and the "opened per-webhook database" line is logged once.

Disclosures:

  • The failure the old code actually shows: two parallel opens of a new file create its tables at once, and one caller gets "table users already exists".
  • Partly verified (read in the driver's source, not tested): the lock loss the issue describes probably does not happen, as SQLite holds a close back while another connection in the process has locks on the file. The fix does not depend on this.
  • Other caches: the archive writer registry creates writers under its own mutex and never opens then discards; the circuit-breaker cache holds no file. No new issue filed.

Model: opus-5-5

Fixes https://git.eeqj.de/sneak/webhooker/issues/291. `GetDB` opened a webhook's event database on a cache miss and only then tried to cache it, so racing callers could each open the file, and the losers closed their copies. - One mutex in `WebhookDBManager` now covers opening. `GetDB` looks in the cache again under it, so a caller that lost the race waits for the first caller's handle; no second handle is opened, closed or leaked. Reading an already cached database takes no lock. - `DeleteDB` holds the mutex until the files are removed, so `GetDB` cannot reopen the file in between. `CloseAll` holds it so an open under way is closed with the rest, not cached afterwards. - Opens of different webhooks now wait for each other; each webhook is opened once per process. - The new test starts many callers on one webhook at once and checks they share one handle and the "opened per-webhook database" line is logged once. Disclosures: - The failure the old code actually shows: two parallel opens of a new file create its tables at once, and one caller gets "table `users` already exists". - Partly verified (read in the driver's source, not tested): the lock loss the issue describes probably does not happen, as SQLite holds a close back while another connection in the process has locks on the file. The fix does not depend on this. - Other caches: the archive writer registry creates writers under its own mutex and never opens then discards; the circuit-breaker cache holds no file. No new issue filed. Model: opus-5-5
clawbot added the needs-review label 2026-09-29 03:23:26 +02:00
clawbot self-assigned this 2026-09-29 03:23:26 +02:00
clawbot added 1 commit 2026-09-29 03:23:26 +02:00
GetDB opened the database on a cache miss and then tried to cache it,
so callers racing on a webhook's first use could each open the file,
and the losers closed their copies. On a new file the parallel opens
also create its tables at the same time, and one caller can fail with
"table already exists".

A mutex now covers the open: GetDB looks in the cache again under it,
then opens and caches. DeleteDB and CloseAll take the same mutex, so
neither runs while an open is under way. Reading an already cached
database takes no lock.

The new test starts many callers on one webhook at once and checks
that exactly one open happened.

Model: opus-5-5
All checks were successful
check / check (push) Successful in 3m44s
You are not authorized to merge this pull request.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue-291-getdb-single-open:issue-291-getdb-single-open
git checkout issue-291-getdb-single-open
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#327