From ac68079bd2b9ed9a663fe18af96cac13e07d0766 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Garapich?= <michal@garapich.pl>
Date: Wed, 30 Mar 2022 00:51:38 +0200
Subject: [PATCH] fix(games): launch orphaned games in a critical section
 (#1523)

---
 .../services/game-launcher.service.spec.ts    | 19 +++++++-----
 src/games/services/game-launcher.service.ts   | 30 ++++++++++++-------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/src/games/services/game-launcher.service.spec.ts b/src/games/services/game-launcher.service.spec.ts
index 324f28f02..9b9277166 100644
--- a/src/games/services/game-launcher.service.spec.ts
+++ b/src/games/services/game-launcher.service.spec.ts
@@ -7,11 +7,14 @@ import { Events } from '@/events/events';
 import { GameServer } from '@/game-servers/models/game-server';
 import { MongoMemoryServer } from 'mongodb-memory-server';
 import { mongooseTestingModule } from '@/utils/testing-mongoose-module';
-import { getModelToken, MongooseModule } from '@nestjs/mongoose';
+import {
+  getConnectionToken,
+  getModelToken,
+  MongooseModule,
+} from '@nestjs/mongoose';
 import { Game, GameDocument, gameSchema } from '../models/game';
-import { Model, Types, Error } from 'mongoose';
+import { Model, Types, Error, Connection } from 'mongoose';
 import { staticGameServerProviderName } from '@/game-servers/providers/static-game-server/static-game-server-provider-name';
-import { NotImplementedError } from '@/game-servers/errors/not-implemented.error';
 
 jest.mock('@/game-servers/services/game-servers.service');
 jest.mock('./games.service');
@@ -26,6 +29,7 @@ describe('GameLauncherService', () => {
   let game: GameDocument;
   let gameModel: Model<GameDocument>;
   let mockGameServer: jest.Mocked<GameServer>;
+  let connection: Connection;
 
   beforeAll(async () => (mongod = await MongoMemoryServer.create()));
   afterAll(async () => await mongod.stop());
@@ -55,6 +59,7 @@ describe('GameLauncherService', () => {
     gameServersService = module.get(GameServersService);
     serverConfiguratorService = module.get(ServerConfiguratorService);
     gameModel = module.get(getModelToken(Game.name));
+    connection = module.get(getConnectionToken());
 
     mockGameServer = {
       id: 'FAKE_GAME_SERVER_ID',
@@ -82,12 +87,11 @@ describe('GameLauncherService', () => {
 
     // @ts-expect-error
     game = await gamesService._createOne();
-    game.gameServer = new Types.ObjectId();
-    await game.save();
   });
 
   afterEach(async () => {
     await gameModel.deleteMany({});
+    await connection.close();
   });
 
   it('should be defined', () => {
@@ -125,9 +129,10 @@ describe('GameLauncherService', () => {
     });
 
     it('should launch orphaned games', async () => {
-      const spy = jest.spyOn(service, 'launch');
       await service.launchOrphanedGames();
-      expect(spy).toHaveBeenCalledWith(game.id);
+      expect(serverConfiguratorService.configureServer).toHaveBeenCalledWith(
+        game.id,
+      );
     });
   });
 });
diff --git a/src/games/services/game-launcher.service.ts b/src/games/services/game-launcher.service.ts
index c3e58d52d..b562a2ab2 100644
--- a/src/games/services/game-launcher.service.ts
+++ b/src/games/services/game-launcher.service.ts
@@ -4,6 +4,7 @@ import { GameServersService } from '@/game-servers/services/game-servers.service
 import { ServerConfiguratorService } from './server-configurator.service';
 import { Cron, CronExpression } from '@nestjs/schedule';
 import { Game } from '../models/game';
+import { Mutex } from 'async-mutex';
 
 /**
  * This service is responsible for launching a single game.
@@ -14,6 +15,7 @@ import { Game } from '../models/game';
 @Injectable()
 export class GameLauncherService {
   private logger = new Logger(GameLauncherService.name);
+  private mutex = new Mutex();
 
   constructor(
     @Inject(forwardRef(() => GamesService)) private gamesService: GamesService,
@@ -28,6 +30,23 @@ export class GameLauncherService {
    * @memberof GameLauncherService
    */
   async launch(gameId: string): Promise<Game> {
+    return await this.mutex.runExclusive(
+      async () => await this.doLaunch(gameId),
+    );
+  }
+
+  @Cron(CronExpression.EVERY_MINUTE)
+  async launchOrphanedGames() {
+    return await this.mutex.runExclusive(async () => {
+      const orphanedGames = await this.gamesService.getOrphanedGames();
+      for (const game of orphanedGames) {
+        this.logger.verbose(`launching game #${game.number}...`);
+        await this.doLaunch(game.id);
+      }
+    });
+  }
+
+  private async doLaunch(gameId: string): Promise<Game> {
     let game = await this.gamesService.getById(gameId);
 
     if (!game.isInProgress()) {
@@ -47,8 +66,8 @@ export class GameLauncherService {
 
       // step 2: obtain logsecret
       const logSecret = await gameServer.getLogsecret();
-      this.logger.debug(`[${gameServer.name}] logsecret is ${game.logSecret}`);
       game = await this.gamesService.update(game.id, { logSecret });
+      this.logger.debug(`[${gameServer.name}] logsecret is ${game.logSecret}`);
 
       // step 3: configure server
       const { connectString, stvConnectString } =
@@ -70,13 +89,4 @@ export class GameLauncherService {
       this.logger.error(`Error launching game #${game.number}: ${error}`);
     }
   }
-
-  @Cron(CronExpression.EVERY_MINUTE) // every minute
-  async launchOrphanedGames() {
-    const orphanedGames = await this.gamesService.getOrphanedGames();
-    for (const game of orphanedGames) {
-      this.logger.verbose(`launching game #${game.number}...`);
-      await this.launch(game.id);
-    }
-  }
 }