Skip to content

Detect a class of race conditions in which the game state has changed between load and save #2798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cgolubi1 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Detect a class of race conditions in which the game state has changed between load and save #2798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/engine/BMInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,15 +924,18 @@ protected function recreate_optRequestArrayArray($game) {
* Save game to database
*
* @param BMGame $game
* @param int $expectedPrevState
*/
protected function save_game(BMGame $game) {
protected function save_game(BMGame $game, int $expectedPrevState) {
// force game to proceed to the latest possible before saving
$game->proceed_to_next_user_action();

// start transaction
self::$conn->beginTransaction();

try {
$prevGame = $this->load_game_parameters($game->gameId, FALSE);
assert($prevGame->gameState == $expectedPrevState, "DB game state changed between load and save");
$this->resolve_random_button_selection($game);
$this->save_basic_game_parameters($game);
$this->save_button_recipes($game);
Expand Down
33 changes: 22 additions & 11 deletions src/engine/BMInterfaceGame.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public function create_game(

// update game state to latest possible
$game = $this->load_game($gameId, NULL, TRUE);
$prevState = $game->gameState;
if (!($game instanceof BMGame)) {
throw new UnexpectedValueException(
"Could not load newly-created game $gameId"
Expand All @@ -103,7 +104,7 @@ public function create_game(
$chatNotice = '[i]Continued from [game=' . $previousGameId . '][i]';
$game->add_chat(-1, $chatNotice);
}
$this->save_game($game);
$this->save_game($game, $prevState);

$this->set_message("Game $gameId created successfully.");
return array('gameId' => $gameId);
Expand Down Expand Up @@ -560,6 +561,7 @@ public function save_join_game_decision($playerId, $gameId, $decision) {
}

$game = $this->load_game($gameId);
$prevState = $game->gameState;

if (BMGameState::CHOOSE_JOIN_GAME != $game->gameState) {
if (('reject' == $decision) &&
Expand Down Expand Up @@ -596,7 +598,7 @@ public function save_join_game_decision($playerId, $gameId, $decision) {
$game->gameState = BMGameState::CANCELLED;
}

$this->save_game($game);
$this->save_game($game, $prevState);

if ($decisionFlag) {
$this->set_message("Joined game $gameId");
Expand Down Expand Up @@ -659,9 +661,10 @@ public function join_open_game($currentPlayerId, $gameId) {
self::$db->update($query, $parameters);

$game = $this->load_game($gameId);
$prevState = $game->gameState;
$player = $game->playerArray[$emptyPlayerIdx];
$player->hasPlayerAcceptedGame = TRUE;
$this->save_game($game);
$this->save_game($game, $prevState);
$this->set_message('Successfully joined game ' . $gameId);

return TRUE;
Expand All @@ -684,6 +687,7 @@ public function join_open_game($currentPlayerId, $gameId) {
public function cancel_open_game($currentPlayerId, $gameId) {
try {
$game = $this->load_game($gameId);
$prevState = $game->gameState;

// check that the current player is involved in this game
$isPlayerInThisGame = ($game->playerArray[0]->playerId == $currentPlayerId);
Expand All @@ -706,7 +710,7 @@ public function cancel_open_game($currentPlayerId, $gameId) {

$game->gameState = BMGameState::CANCELLED;

$this->save_game($game);
$this->save_game($game, $prevState);
$this->set_message("Successfully cancelled game $gameId");

return TRUE;
Expand Down Expand Up @@ -785,7 +789,8 @@ public function select_button(
self::$db->update($query, $parameters);

$game = $this->load_game($gameId);
$this->save_game($game);
$prevState = $game->gameState;
$this->save_game($game, $prevState);

return TRUE;
} catch (Exception $e) {
Expand Down Expand Up @@ -816,6 +821,7 @@ public function submit_die_values(
) {
try {
$game = $this->load_game($gameId);
$prevState = $game->gameState;
$currentPlayerIdx = array_search($playerId, $game->playerIdArray);

// check that the timestamp and the game state are correct, and that
Expand Down Expand Up @@ -875,7 +881,7 @@ public function submit_die_values(
if ((FALSE == $game->playerArray[$currentPlayerIdx]->waitingOnAction) ||
($game->gameState > BMGameState::SPECIFY_DICE) ||
($game->roundNumber > $roundNumber)) {
$this->save_game($game);
$this->save_game($game, $prevState);
$this->set_message('Successfully set die sizes');
return TRUE;
} else {
Expand Down Expand Up @@ -967,6 +973,7 @@ protected function set_option_values($optionValueArray, $currentPlayerIdx, $game
public function submit_turn($args) {
try {
$game = $this->load_game($args['game']);
$prevState = $game->gameState;

if (!$this->is_action_current(
$game,
Expand Down Expand Up @@ -1024,7 +1031,7 @@ public function submit_turn($args) {
return NULL;
}

$this->save_game($game);
$this->save_game($game, $prevState);

// On success, don't set a message, because one will be set from the action log
return TRUE;
Expand Down Expand Up @@ -1151,6 +1158,7 @@ public function react_to_auxiliary(
$this->set_message('Invalid game');
return FALSE;
}
$prevState = $game->gameState;
if (!$this->is_action_current(
$game,
BMGameState::CHOOSE_AUXILIARY_DICE,
Expand Down Expand Up @@ -1204,7 +1212,7 @@ public function react_to_auxiliary(
$this->set_message('Invalid response to auxiliary choice.');
return FALSE;
}
$this->save_game($game);
$this->save_game($game, $prevState);
return TRUE;
} catch (Exception $e) {
error_log(
Expand Down Expand Up @@ -1264,6 +1272,7 @@ public function react_to_reserve(
) {
try {
$game = $this->load_game($gameId);
$prevState = $game->gameState;
if (!$this->is_action_current(
$game,
BMGameState::CHOOSE_RESERVE_DICE,
Expand Down Expand Up @@ -1308,7 +1317,7 @@ public function react_to_reserve(
return FALSE;
}

$this->save_game($game);
$this->save_game($game, $prevState);

return TRUE;
} catch (Exception $e) {
Expand Down Expand Up @@ -1368,6 +1377,7 @@ public function react_to_initiative(
) {
try {
$game = $this->load_game($gameId);
$prevState = $game->gameState;
if (!$this->is_action_current(
$game,
BMGameState::REACT_TO_INITIATIVE,
Expand Down Expand Up @@ -1412,7 +1422,7 @@ public function react_to_initiative(

$isSuccessful = $game->react_to_initiative($argArray);
if ($isSuccessful) {
$this->save_game($game);
$this->save_game($game, $prevState);

if ($isSuccessful['gainedInitiative']) {
$this->set_message('Successfully gained initiative');
Expand Down Expand Up @@ -1480,6 +1490,7 @@ public function adjust_fire(
) {
try {
$game = $this->load_game($gameId);
$prevState = $game->gameState;
if (!$this->is_action_current(
$game,
BMGameState::ADJUST_FIRE_DICE,
Expand Down Expand Up @@ -1540,7 +1551,7 @@ public function adjust_fire(
);
}

$this->save_game($game);
$this->save_game($game, $prevState);
} else {
$this->set_message('Invalid fire turndown');
}
Expand Down
10 changes: 5 additions & 5 deletions test/src/engine/BMInterfaceGameTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ public function test_create_game_with_one_random_button_and_one_unspecified() {
$this->assertEmpty($game->playerArray[1]->button);
$this->assertFalse($game->playerArray[1]->isButtonChoiceRandom);

self::save_game($game);
self::save_game($game, $game->gameState);
$game = self::load_game($gameId);
$this->assertEmpty($game->playerArray[0]->button);
$this->assertTrue($game->playerArray[0]->isButtonChoiceRandom);
Expand Down Expand Up @@ -1088,7 +1088,7 @@ public function test_react_to_reserve_decline() {
$playerArray[1]->activeDieArray = array();
$game->playerArray = $playerArray;

self::save_game($game);
self::save_game($game, $game->gameState);
$game = self::load_game($game->gameId);

$this->assertEquals(BMGameState::CHOOSE_RESERVE_DICE, $game->gameState);
Expand Down Expand Up @@ -1175,7 +1175,7 @@ public function test_react_to_reserve_add() {
$playerArray[1]->activeDieArray = array();
$game->playerArray = $playerArray;

self::save_game($game);
self::save_game($game, $game->gameState);
$game = self::load_game($game->gameId);

$this->assertEquals(BMGameState::CHOOSE_RESERVE_DICE, $game->gameState);
Expand Down Expand Up @@ -1256,7 +1256,7 @@ function test_fire() {

$game->swingValueArrayArray = array(array('X' => 17), array('X' => 5));

self::save_game($game);
self::save_game($game, $game->gameState);
$game = self::load_game($gameId);

$this->assertEquals(BMGameState::START_TURN, $game->gameState);
Expand Down Expand Up @@ -1298,7 +1298,7 @@ function test_fire() {
array(0), // defenderAttackDieIdxArray
'Power'); // attackType

self::save_game($game);
self::save_game($game, $game->gameState);
$game = self::load_game($game->gameId);

$this->assertEquals(array(TRUE, FALSE), $game->waitingOnActionArray);
Expand Down
Loading