Skip to content

Commit fabc48b

Browse files
cgolubi1cgolubi1
committed
Detect a class of race conditions in which the game state has changed between load and save
1 parent 651755b commit fabc48b

5 files changed

Lines changed: 97 additions & 83 deletions

File tree

‎src/engine/BMInterface.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -924,15 +924,18 @@ protected function recreate_optRequestArrayArray($game) {
924924
* Save game to database
925925
*
926926
* @param BMGame $game
927+
* @param int $expectedPrevState
927928
*/
928-
protected function save_game(BMGame $game) {
929+
protected function save_game(BMGame $game, int $expectedPrevState) {
929930
// force game to proceed to the latest possible before saving
930931
$game->proceed_to_next_user_action();
931932

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

935936
try {
937+
$prevGame = $this->load_game_parameters($game->gameId, FALSE);
938+
assert ($prevGame->gameState == $expectedPrevState, "Game state unexpectedly changed before move could be saved");
936939
$this->resolve_random_button_selection($game);
937940
$this->save_basic_game_parameters($game);
938941
$this->save_button_recipes($game);

‎src/engine/BMInterfaceGame.php‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ public function create_game(
8787

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

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

562563
$game = $this->load_game($gameId);
564+
$prevState = $game->gameState;
563565

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

599-
$this->save_game($game);
601+
$this->save_game($game, $prevState);
600602

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

661663
$game = $this->load_game($gameId);
664+
$prevState = $game->gameState;
662665
$player = $game->playerArray[$emptyPlayerIdx];
663666
$player->hasPlayerAcceptedGame = TRUE;
664-
$this->save_game($game);
667+
$this->save_game($game, $prevState);
665668
$this->set_message('Successfully joined game ' . $gameId);
666669

667670
return TRUE;
@@ -684,6 +687,7 @@ public function join_open_game($currentPlayerId, $gameId) {
684687
public function cancel_open_game($currentPlayerId, $gameId) {
685688
try {
686689
$game = $this->load_game($gameId);
690+
$prevState = $game->gameState;
687691

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

707711
$game->gameState = BMGameState::CANCELLED;
708712

709-
$this->save_game($game);
713+
$this->save_game($game, $prevState);
710714
$this->set_message("Successfully cancelled game $gameId");
711715

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

787791
$game = $this->load_game($gameId);
788-
$this->save_game($game);
792+
$prevState = $game->gameState;
793+
$this->save_game($game, $prevState);
789794

790795
return TRUE;
791796
} catch (Exception $e) {
@@ -816,6 +821,7 @@ public function submit_die_values(
816821
) {
817822
try {
818823
$game = $this->load_game($gameId);
824+
$prevState = $game->gameState;
819825
$currentPlayerIdx = array_search($playerId, $game->playerIdArray);
820826

821827
// check that the timestamp and the game state are correct, and that
@@ -875,7 +881,7 @@ public function submit_die_values(
875881
if ((FALSE == $game->playerArray[$currentPlayerIdx]->waitingOnAction) ||
876882
($game->gameState > BMGameState::SPECIFY_DICE) ||
877883
($game->roundNumber > $roundNumber)) {
878-
$this->save_game($game);
884+
$this->save_game($game, $prevState);
879885
$this->set_message('Successfully set die sizes');
880886
return TRUE;
881887
} else {
@@ -967,6 +973,7 @@ protected function set_option_values($optionValueArray, $currentPlayerIdx, $game
967973
public function submit_turn($args) {
968974
try {
969975
$game = $this->load_game($args['game']);
976+
$prevState = $game->gameState;
970977

971978
if (!$this->is_action_current(
972979
$game,
@@ -1024,7 +1031,7 @@ public function submit_turn($args) {
10241031
return NULL;
10251032
}
10261033

1027-
$this->save_game($game);
1034+
$this->save_game($game, $prevState);
10281035

10291036
// On success, don't set a message, because one will be set from the action log
10301037
return TRUE;
@@ -1151,6 +1158,7 @@ public function react_to_auxiliary(
11511158
$this->set_message('Invalid game');
11521159
return FALSE;
11531160
}
1161+
$prevState = $game->gameState;
11541162
if (!$this->is_action_current(
11551163
$game,
11561164
BMGameState::CHOOSE_AUXILIARY_DICE,
@@ -1204,7 +1212,7 @@ public function react_to_auxiliary(
12041212
$this->set_message('Invalid response to auxiliary choice.');
12051213
return FALSE;
12061214
}
1207-
$this->save_game($game);
1215+
$this->save_game($game, $prevState);
12081216
return TRUE;
12091217
} catch (Exception $e) {
12101218
error_log(
@@ -1264,6 +1272,7 @@ public function react_to_reserve(
12641272
) {
12651273
try {
12661274
$game = $this->load_game($gameId);
1275+
$prevState = $game->gameState;
12671276
if (!$this->is_action_current(
12681277
$game,
12691278
BMGameState::CHOOSE_RESERVE_DICE,
@@ -1308,7 +1317,7 @@ public function react_to_reserve(
13081317
return FALSE;
13091318
}
13101319

1311-
$this->save_game($game);
1320+
$this->save_game($game, $prevState);
13121321

13131322
return TRUE;
13141323
} catch (Exception $e) {
@@ -1368,6 +1377,7 @@ public function react_to_initiative(
13681377
) {
13691378
try {
13701379
$game = $this->load_game($gameId);
1380+
$prevState = $game->gameState;
13711381
if (!$this->is_action_current(
13721382
$game,
13731383
BMGameState::REACT_TO_INITIATIVE,
@@ -1412,7 +1422,7 @@ public function react_to_initiative(
14121422

14131423
$isSuccessful = $game->react_to_initiative($argArray);
14141424
if ($isSuccessful) {
1415-
$this->save_game($game);
1425+
$this->save_game($game, $prevState);
14161426

14171427
if ($isSuccessful['gainedInitiative']) {
14181428
$this->set_message('Successfully gained initiative');
@@ -1480,6 +1490,7 @@ public function adjust_fire(
14801490
) {
14811491
try {
14821492
$game = $this->load_game($gameId);
1493+
$prevState = $game->gameState;
14831494
if (!$this->is_action_current(
14841495
$game,
14851496
BMGameState::ADJUST_FIRE_DICE,
@@ -1540,7 +1551,7 @@ public function adjust_fire(
15401551
);
15411552
}
15421553

1543-
$this->save_game($game);
1554+
$this->save_game($game, $prevState);
15441555
} else {
15451556
$this->set_message('Invalid fire turndown');
15461557
}

‎test/src/engine/BMInterfaceGameTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ public function test_create_game_with_one_random_button_and_one_unspecified() {
397397
$this->assertEmpty($game->playerArray[1]->button);
398398
$this->assertFalse($game->playerArray[1]->isButtonChoiceRandom);
399399

400-
self::save_game($game);
400+
self::save_game($game, $game->gameState);
401401
$game = self::load_game($gameId);
402402
$this->assertEmpty($game->playerArray[0]->button);
403403
$this->assertTrue($game->playerArray[0]->isButtonChoiceRandom);
@@ -1088,7 +1088,7 @@ public function test_react_to_reserve_decline() {
10881088
$playerArray[1]->activeDieArray = array();
10891089
$game->playerArray = $playerArray;
10901090

1091-
self::save_game($game);
1091+
self::save_game($game, $game->gameState);
10921092
$game = self::load_game($game->gameId);
10931093

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

1178-
self::save_game($game);
1178+
self::save_game($game, $game->gameState);
11791179
$game = self::load_game($game->gameId);
11801180

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

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

1259-
self::save_game($game);
1259+
self::save_game($game, $game->gameState);
12601260
$game = self::load_game($gameId);
12611261

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

1301-
self::save_game($game);
1301+
self::save_game($game, $game->gameState);
13021302
$game = self::load_game($game->gameId);
13031303

13041304
$this->assertEquals(array(TRUE, FALSE), $game->waitingOnActionArray);

0 commit comments

Comments
 (0)