create_ticket_api.php: dedup/escalation race can duplicate notifications or drop a real escalation #35

New Issue

2026-08-31T21:29:43-04:00

jared commented

2026-08-31 21:29:43 -04:00

Severity: Medium

create_ticket_api.php (~lines 253-347 and ~457-470): the dedup lookup (SELECT ... WHERE hash = ?) and the subsequent priority-escalation UPDATE run with no transaction or row lock.

Two related failure modes:

Two concurrent hwmonDaemon requests carrying the same dedup hash (e.g. overlapping monitoring runs) can both read the same pre-update snapshot and each independently conclude "priority escalated," each posting its own hwmonDaemon raised priority... comment and firing its own Matrix notification for what is really one escalation event.
On the rarer SELECT→INSERT race (two near-simultaneous creates for the same dedup hash), the loser gets {success:false, error:'Duplicate ticket'} and the request is dropped entirely — it never falls back to applying the priority-escalation/description-refresh logic the normal (non-race) duplicate path performs, so a genuinely-worsening alert that loses the race is silently discarded instead of escalating the existing ticket.

Fix: Wrap the dedup-check + escalate/reopen/insert sequence in a transaction with a row lock (SELECT ... FOR UPDATE) on the matched ticket (or a unique constraint + INSERT ... ON DUPLICATE KEY pattern) so concurrent requests for the same hash serialize correctly instead of racing.

**Severity:** Medium `create_ticket_api.php` (~lines 253-347 and ~457-470): the dedup lookup (`SELECT ... WHERE hash = ?`) and the subsequent priority-escalation `UPDATE` run with no transaction or row lock. **Two related failure modes:** 1. Two concurrent hwmonDaemon requests carrying the same dedup hash (e.g. overlapping monitoring runs) can both read the same pre-update snapshot and each independently conclude "priority escalated," each posting its own `hwmonDaemon raised priority...` comment and firing its own Matrix notification for what is really one escalation event. 2. On the rarer SELECT→INSERT race (two near-simultaneous *creates* for the same dedup hash), the loser gets `{success:false, error:'Duplicate ticket'}` and the request is dropped entirely — it never falls back to applying the priority-escalation/description-refresh logic the normal (non-race) duplicate path performs, so a genuinely-worsening alert that loses the race is silently discarded instead of escalating the existing ticket. **Fix:** Wrap the dedup-check + escalate/reopen/insert sequence in a transaction with a row lock (`SELECT ... FOR UPDATE`) on the matched ticket (or a unique constraint + `INSERT ... ON DUPLICATE KEY` pattern) so concurrent requests for the same hash serialize correctly instead of racing.

jared added the concurrency notifications priority/medium labels 2026-09-08 10:15:44 -04:00

jared referenced this issue from a commit

2026-09-11 21:42:01 -04:00

Serialize dedup-hash lookups in create_ticket_api.php (#35)

jared referenced this issue from a commit

2026-09-11 21:45:26 -04:00

Merge development into main: concurrency/atomicity fixes (#34, #35, #37)

jared commented

2026-09-11 21:50:57 -04:00

Fixed in commit 3778a59 (merged to main in 3ab33d8).

Two concurrent hwmonDaemon reports carrying the same dedup hash could both read the same pre-update ticket snapshot and each independently apply a priority escalation (losing one), or both attempt to INSERT a new ticket for a not-yet-existing hash and have the loser's request dropped with a 'Duplicate ticket' error instead of falling through to the update/escalate path.

Fix: wrapped the hash lookup through the update-or-insert in one transaction, with the lookup taking SELECT ... FOR UPDATE. For an existing row this serializes the read-modify-write. For a not-yet-existing hash, testing showed InnoDB's gap lock there is shared rather than exclusive, so two concurrent inserts can both reach the INSERT and deadlock (error 1213) instead of one cleanly blocking on the other's row — added a bounded retry (once) on that deadlock (or a lock-wait-timeout, 1205) so the retry's own SELECT finds the winner's committed row and takes the update path instead of erroring.

Verified against real MariaDB with two concurrent OS processes for both scenarios: (1) same existing active ticket — second process blocked ~1.1s on the held row lock, then correctly escalated from the first's committed priority; (2) same not-yet-existing hash — reproduced the 1213 deadlock deterministically (5/5 runs) with the pre-retry code, then confirmed the retry resolves it every time (5/5), leaving exactly one ticket row created and no dropped/erroring request.

Fixed in commit 3778a59 (merged to main in 3ab33d8). Two concurrent hwmonDaemon reports carrying the same dedup hash could both read the same pre-update ticket snapshot and each independently apply a priority escalation (losing one), or both attempt to INSERT a new ticket for a not-yet-existing hash and have the loser's request dropped with a 'Duplicate ticket' error instead of falling through to the update/escalate path. Fix: wrapped the hash lookup through the update-or-insert in one transaction, with the lookup taking SELECT ... FOR UPDATE. For an existing row this serializes the read-modify-write. For a not-yet-existing hash, testing showed InnoDB's gap lock there is shared rather than exclusive, so two concurrent inserts can both reach the INSERT and deadlock (error 1213) instead of one cleanly blocking on the other's row — added a bounded retry (once) on that deadlock (or a lock-wait-timeout, 1205) so the retry's own SELECT finds the winner's committed row and takes the update path instead of erroring. Verified against real MariaDB with two concurrent OS processes for both scenarios: (1) same existing active ticket — second process blocked ~1.1s on the held row lock, then correctly escalated from the first's committed priority; (2) same not-yet-existing hash — reproduced the 1213 deadlock deterministically (5/5 runs) with the pre-retry code, then confirmed the retry resolves it every time (5/5), leaving exactly one ticket row created and no dropped/erroring request.

jared closed this issue

2026-09-11 21:50:57 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: LotusGuild/tinker_tickets#35