diff --git a/services/nginx/app/modules/selfserve/traits/selfserve_lane_command_t.php b/services/nginx/app/modules/selfserve/traits/selfserve_lane_command_t.php index 2efe709c..825f16ba 100644 --- a/services/nginx/app/modules/selfserve/traits/selfserve_lane_command_t.php +++ b/services/nginx/app/modules/selfserve/traits/selfserve_lane_command_t.php @@ -38,6 +38,49 @@ use objects\department_variables_o; trait selfserve_lane_command_t { + + /** + * Acquire an atomic per-lane START lock before performing physical side effects. + */ + protected function acquireLaneStartCommandLock(): string + { + if (!defined('redis') || !method_exists(redis, 'set_if_absent_with_expiration')) { + throw new \RuntimeException('Cannot start lane: START lock is unavailable.'); + } + + $token = bin2hex(random_bytes(16)); + $lock_key = $this->getLaneStartCommandLockKey(); + if (!redis->set_if_absent_with_expiration($lock_key, $token, 30)) { + throw new \RuntimeException("Cannot start lane: Lane is not available."); + } + + return $token; + } + + protected function releaseLaneStartCommandLock(string $token): void + { + if (!defined('redis')) { + return; + } + + $lock_key = $this->getLaneStartCommandLockKey(); + try { + if (method_exists(redis, 'get') && redis->get($lock_key) !== $token) { + return; + } + if (method_exists(redis, 'delete')) { + redis->delete($lock_key); + } + } catch (\Throwable) { + // The lock has a short TTL, so release failures must not mask START results. + } + } + + protected function getLaneStartCommandLockKey(): string + { + return 'selfserve_lane_start_command_lock_' . (int)$this->id; + } + /** * Determine if the lane and its department have self-serve enabled. * This method is intentionally protected to allow tests to override @@ -509,38 +552,48 @@ trait selfserve_lane_command_t // Validate customer number if (!is_numeric($customer_number) || (int)$customer_number <= 0) throw new \InvalidArgumentException("Invalid customer number: " . $customer_number); if (!(new users_o())->getUserByCustomerNumber((int)$customer_number)->exists()) throw new \InvalidArgumentException("Customer number does not exist: " . $customer_number); - $previous_customer_number = $this->getCustomerNumber(); - $previous_license_plate = $this->getLicensePlate(); - // Set the customer number and license plate - $this->setCustomerNumber($customer_number); - $this->setLicensePlate($license_plate); + $start_lock_token = $this->acquireLaneStartCommandLock(); try { - // Open the entrance port before marking the lane occupied. Gateway timeouts are - // ambiguous because the relay may already have received the pulse. - $this->openEntrancePortForWashStart(); - } catch (\Throwable $e) { - $this->setCustomerNumber($previous_customer_number); - $this->setLicensePlate($previous_license_plate); - $this->setLaneState(selfserve_lane_state::IDLE); - throw $e; + // Re-check availability after taking the START lock so concurrent requests cannot + // both pass the preflight check and trigger the physical entrance relay. + if (!$this->getLaneStatus()->equals(selfserve_lane_status::AVAILABLE)) throw new \RuntimeException("Cannot start lane: Lane is not available."); + $previous_customer_number = $this->getCustomerNumber(); + $previous_license_plate = $this->getLicensePlate(); + $previous_status = $this->getLaneStatus(); + // Set the customer number and license plate + $this->setCustomerNumber($customer_number); + $this->setLicensePlate($license_plate); + // Mark the lane occupied before any physical entrance relay side effects. + $this->setLaneStatus(selfserve_lane_status::OCCUPIED); + try { + // Gateway timeouts are ambiguous because the relay may already have received + // the pulse, so openEntrancePortForWashStart() reports them and continues. + $this->openEntrancePortForWashStart(); + } catch (\Throwable $e) { + $this->setCustomerNumber($previous_customer_number); + $this->setLicensePlate($previous_license_plate); + $this->setLaneStatus($previous_status); + $this->setLaneState(selfserve_lane_state::IDLE); + throw $e; + } + // Set the lane state to IN_WASH + $this->setLaneState(selfserve_lane_state::IN_WASH); + // Start the wash timer + $this->setWashStartTime(time()); + $this->runPublishedStudioActions( + selfserve_studio_actions::EVENT_WASH_START_COMMAND, + $this->resolveSelfServeActionWashModeForStart(), + [ + 'customer_number' => (int)$customer_number, + 'reg' => $license_plate, + ] + ); + $this->runRelaySideEffectsForWashStart($arguments); + // Log the lane start event + $this->logLaneAction(selfserve_lane_log_action::START_WASH); + } finally { + $this->releaseLaneStartCommandLock($start_lock_token); } - // Set the lane status to OCCUPIED when started - $this->setLaneStatus(selfserve_lane_status::OCCUPIED); - // Set the lane state to IN_WASH - $this->setLaneState(selfserve_lane_state::IN_WASH); - // Start the wash timer - $this->setWashStartTime(time()); - $this->runPublishedStudioActions( - selfserve_studio_actions::EVENT_WASH_START_COMMAND, - $this->resolveSelfServeActionWashModeForStart(), - [ - 'customer_number' => (int)$customer_number, - 'reg' => $license_plate, - ] - ); - $this->runRelaySideEffectsForWashStart($arguments); - // Log the lane start event - $this->logLaneAction(selfserve_lane_log_action::START_WASH); break; case selfserve_lane_command::STOP: // Require lane to be occupied before stopping diff --git a/services/nginx/app/tests/Unit/Selfserve/SelfserveLaneStartRaceLockWiringTest.php b/services/nginx/app/tests/Unit/Selfserve/SelfserveLaneStartRaceLockWiringTest.php new file mode 100644 index 00000000..ee91df43 --- /dev/null +++ b/services/nginx/app/tests/Unit/Selfserve/SelfserveLaneStartRaceLockWiringTest.php @@ -0,0 +1,28 @@ +not->toBeFalse(); + expect($commandTrait)->toContain('set_if_absent_with_expiration'); + + $startCaseOffset = strpos($commandTrait, 'case selfserve_lane_command::START:'); + expect($startCaseOffset)->not->toBeFalse(); + + $startCase = substr($commandTrait, (int)$startCaseOffset, 3500); + $lockOffset = strpos($startCase, '$start_lock_token = $this->acquireLaneStartCommandLock();'); + $occupiedOffset = strpos($startCase, '$this->setLaneStatus(selfserve_lane_status::OCCUPIED);'); + $openOffset = strpos($startCase, '$this->openEntrancePortForWashStart();'); + $finallyOffset = strpos($startCase, '} finally {'); + $releaseOffset = strpos($startCase, '$this->releaseLaneStartCommandLock($start_lock_token);'); + + expect($lockOffset)->not->toBeFalse() + ->and($occupiedOffset)->not->toBeFalse() + ->and($openOffset)->not->toBeFalse() + ->and($finallyOffset)->not->toBeFalse() + ->and($releaseOffset)->not->toBeFalse() + ->and($lockOffset)->toBeLessThan($occupiedOffset) + ->and($occupiedOffset)->toBeLessThan($openOffset) + ->and($openOffset)->toBeLessThan($finallyOffset) + ->and($finallyOffset)->toBeLessThan($releaseOffset); +});