A suspended hold resumes with a brand new window, so cancelling a checkout repeatedly holds a sold-out slot indefinitely

A hold does not resume with the time it had left when the checkout froze it. It starts again from zero.

TransactionManager::unlockTransaction() calls rearmHolds(), which calls BookingManager::renewHold() on every still-held line. renewHold() resolves its deadline through holdDeadline($resource, NULL), which is the request time plus the resource hold TTL: a full fresh window, unrelated to what the line had before. Nothing records the remaining time either, because suspendHold() writes a NULL hold_expires, so the figure a correct resume would need is destroyed at the moment the lock is taken.

The basket clock can therefore be reset at will. A visitor goes to payment, cancels at the provider, and every line is back on the clock with a whole new window. Repeat as often as they like. Where the hold TTL is the only thing returning unwanted places to sale, one visitor can keep a sold-out slot out of circulation for as long as they keep clicking, and nothing about it reads as abuse: each cancellation on its own is an ordinary event the system is right to honor.

Every path that unlocks an order reaches it: kessai.failed, kessai.cancelled and kessai.expired all land on unlockTransaction() through OrderLockSubscriber. #3614424: The checkout amount is computed before the order is locked, so the basket can still change between the two adds a cheaper one, since a checkout that closes without charging unlocks the same way without the visitor ever reaching the provider.

This is the safety half of #3614282: Show how long a cart is held, warn before it lapses, and let the visitor extend it, which asked for a renew the visitor can trigger deliberately and said in the same breath that it needed a cap on total lifetime so an abandoned tab cannot hold beds forever. The renew capability arrived through other work on the same code and was wired straight into the unlock path; the cap never did. So the module now has exactly the unbounded renew that cap was written to prevent. That issue also states that nothing in the module ever extends a hold, which stopped being true once renewHold() and rearmHolds() landed, and it needs correcting.

Not what #3614422: An abandoned checkout leaves its holds suspended for ever: locked_expires is written and never read fixed. That closed the opposite case, a checkout abandoned mid-lock leaving its holds suspended for ever, by reading locked_expires and sweeping the lapsed lock. The sweep only fires on a lock that outlives its deadline, and this loop never produces one: each cancellation resolves the checkout normally.

Proposal: record the deadline the hold had as it is suspended, and put that same deadline back when the checkout ends without a payment. Not the remaining seconds measured again from the moment of resuming: that would pause the clock, and pausing is the same exploit at a slower pace, since a visitor could open the payment page, sit on it for ten minutes and cancel, and come back with those ten minutes handed to them for nothing. The clock has to keep running, so time spent at the payment page is time spent, and going to payment and back wins nothing at all.

A line whose deadline passed during the checkout is restored in the past and the next sweep releases it, which is the honest outcome: the visitor had their window and spent it waiting. A hold that never lapses keeps not lapsing, since there was no deadline to record. Suspending an already suspended hold must not overwrite the record with the NULL it now reads, or the hold resumes with no deadline at all and nothing ever reclaims it.

With the deadline unchanged across a checkout there is nothing left to cap for this bug, so no new setting is needed here. renewHold() keeps its present meaning for the deliberate extension #3614282: Show how long a cart is held, warn before it lapses, and let the visitor extend it will build, with the total-lifetime cap that issue already specifies, and the resume path stops borrowing it.

Correction to the severity stated above. "Indefinitely" holds for a site running yoyaku without a workflow, where nothing else bounds the basket. On an orchestra-driven site, which is the shipped configuration, the payment node carries its own timeout and it is anchored to the node, so re-entering the step after a cancellation does not restart it: the run reaches the timeout outcome and releases the order however many times the visitor cancels. The reset this fixed was real, and the fix stands, but it was bounded there rather than unbounded.