Put two bookers in flight at once, and guard the sort that stops them deadlocking

A hold opens a transaction, takes the slot row with SELECT FOR UPDATE, and saves its booking lines inside it. Saving an entity deletes that entity's row from cache_entity, and that delete is not deferred: ContentEntityStorageBase::resetCache() calls the cache backend straight away, from inside the transaction. Cache TAG invalidation is already safe, because core's CacheTagsChecksumTrait checks inTransaction() and defers the write to a post-commit callback; the entity cache rows have no such protection.

This issue was opened on the suspicion that two genuine concurrent bookers could deadlock on those deletes, and it said a purpose-built two-booker test should come before anything else. That test now exists, and it found something other than what was expected.

What the test does

Two real HTTP requests are put in flight together, because no kernel test can ask this: holds in one process run one after the other, and a second connection would block on the first rather than race it. Four shapes are fired, twelve rounds each, with every request kept inside its transaction for 250 milliseconds so the window is widened by roughly three orders of magnitude over natural timing.

• Two bookers on different slots, the shape this issue was about.
• Two bookers on the same slot, the control: the slot lock serialises them.
• One basket asked twice, two tabs or a click that arrived twice.
• Two group holds naming the same two slots in opposite orders.

The finding: the deadlock is real, and one line prevents it

The first three shapes are SYMMETRIC. Both bookers do the same work in the same order, so they can queue behind one another but never wait on one another. A cycle needs one side taking A then B while the other takes B then A, and a booker can only do that by holding a group that names several slots at once. That is the fourth shape, and it is the only one that can cycle at all.

The engine already sorts the slot ids before locking them, precisely for this. That sort was verified by switching it off. With sort($slot_ids) commented out in BookingManager::hold(), the fourth shape deadlocked at round 7 of 12, MySQL killing one side on the second slot's locking read with SQLSTATE[40001], 1213 Deadlock found. With the sort restored, the same pairs pass on the same machine. Same rig, opposite result.

So two genuine concurrent bookers CAN deadlock here, and a single line of code is the whole defence. It is now guarded by a test, so removing it turns something red instead of turning into an intermittent lost click that looks like anything but a locking bug.

What was NOT found: the entity cache deletes

No collision on the cache_entity deletes, which is the mechanism this issue was opened about. Twelve rounds a shape as committed, plus a one-off run at forty rounds and 240 concurrent pairs, all clean, with the entity cache verified warm on every round so the deletes were hitting rows that exist rather than rows that were never written.

That is a bounded result and should not be read as more. It says the shapes tested do not cycle on those deletes; it does not prove no arrangement of bookers ever could. An explanation consistent with it, offered as reasoning rather than measurement: a hold deletes one cache row per entity save, one cid per statement, and a single-lock deleter cannot be half of a cycle. The only multi-cid deleter in production is the layout reader, on an administrative path no booker touches.

Worth recording plainly, because it was blurred once already in discussion: the collision observed on #3616956: Stop a click made while a request is out from being lost under CI load was between a hold and a POLLING TEST, never between two bookers. That one was properly evidenced, reproduced three times under deliberate contention against 100 clean samples with the second writer removed. An attempt to re-stage that same shape here did not reproduce it on an idle machine, which is why the calibration below uses a constructed cycle instead.

Why the quiet shapes are worth reading at all

A test reporting an absence is worth what its calibration is worth, so three things are asserted every round rather than assumed: that the requests really overlapped by the server's own clock, and if they did not the class SKIPS rather than passes, because a server answering one request at a time cannot be asked this question; that the entity cache really was warm; and that the transaction really stayed open, asserted as a floor and never as an exact figure. A fifth test provokes a deadlock deliberately and insists it comes back named, because a class never seen to report a collision cannot be told apart from one unable to.

What is left, and it is not this issue

The other half stands untouched by all of the above, because it never rested on a deadlock: those cache deletes are still avoidable work inside the one lock a rush contends on, which #3615593: Load test the booking path: what it bears, and what gives way first measured as the queue at 40 to 80 bookers. Two candidate changes were discussed. Opting the booking and the transaction out of the persistent entity cache, measured as costing nothing: every query budget class still passes unchanged. Or deferring the delete past the commit by decorating cache.entity, which is what core already does for tags via addPostTransactionCallback(), and which is more general but carries the staleness hazard core names in the same place. A dedicated lock anchor, #3616896: Seat a party under the hold's own locks, so a rush is served rather than refused, does not address this: it changes what is locked, not what the transaction then writes.

That work is now separated out, so this issue closes on the investigation and the guard it produced, and the cache-delete change is argued on its own terms with its own measurement.

AI-Generated: Yes (Claude Code wrote the test and the test module, ran the comparison with the sort switched off and back on, and drafted this summary; I reviewed it before posting.)