From b7f7ed89a62a82e84866413e772b52e164861a66 Mon Sep 17 00:00:00 2001
From: Will Hunt <will@half-shot.uk>
Date: Sun, 19 May 2019 20:31:07 +0100
Subject: [PATCH] Fix channel locking, *again*

---
 src/bot.ts              | 41 +++++++++++++++++++++--------------------
 test/test_discordbot.ts | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/src/bot.ts b/src/bot.ts
index 0d868bf..a44459c 100644
--- a/src/bot.ts
+++ b/src/bot.ts
@@ -81,8 +81,8 @@ export class DiscordBot {
 
     /* Handles messages queued up to be sent to matrix from discord. */
     private discordMessageQueue: { [channelId: string]: Promise<void> };
-    private channelLocks: { [channelId: string]: {p: Promise<{}>, i: NodeJS.Timeout|null, r: (() => void)|null} };
-
+    private channelLocks: Map<string,{i: NodeJS.Timeout|null, r: (() => void)|null}>;
+    private channelLockPromises: Map<string, Promise<{}>>;
     constructor(
         private botUserId: string,
         private config: DiscordBridgeConfig,
@@ -106,7 +106,8 @@ export class DiscordBot {
         // init vars
         this.sentMessages = [];
         this.discordMessageQueue = {};
-        this.channelLocks = {};
+        this.channelLocks = new Map();
+        this.channelLockPromises = new Map();
         this.lastEventIds = {};
     }
 
@@ -139,39 +140,39 @@ export class DiscordBot {
     }
 
     public lockChannel(channel: Discord.Channel) {
-        if (this.channelLocks[channel.id]) {
+        if (this.channelLocks.has(channel.id)) {
             return;
         }
 
+        this.channelLocks[channel.id] = {i: null, r: null, p: null};
         const p = new Promise((resolve) => {
-            if (!this.channelLocks[channel.id]) {
-                resolve();
-                return;
-            }
-            const i = setInterval(() => {
+            const i = setTimeout(() => {
                 log.warn(`Lock on channel ${channel.id} expired. Discord is lagging behind?`);
-                resolve();
+                this.unlockChannel(channel);
             }, this.config.limits.discordSendDelay);
-            this.channelLocks[channel.id].i = i;
-            this.channelLocks[channel.id].r = resolve;
+            const o = Object.assign({r: resolve, i, p: null}, this.channelLocks.get(channel.id) || {});
+            this.channelLocks.set(channel.id, o);
         });
-
-        this.channelLocks[channel.id] = {i: null, r: null, p};
+        this.channelLockPromises.set(channel.id, p);
     }
 
     public unlockChannel(channel: Discord.Channel) {
-        const lock = this.channelLocks[channel.id];
-        if (lock && lock.i !== null) {
+        if (!this.channelLocks.has(channel.id)) {
+            return;
+        }
+        const lock = this.channelLocks.get(channel.id)!;
+        if (lock.i !== null) {
             lock.r!();
             clearTimeout(lock.i);
         }
-        delete this.channelLocks[channel.id];
+        this.channelLocks.delete(channel.id);
+        this.channelLockPromises.delete(channel.id);
     }
 
     public async waitUnlock(channel: Discord.Channel) {
-        const lock = this.channelLocks[channel.id];
-        if (lock) {
-            await lock.p;
+        const promise = this.channelLockPromises.get(channel.id);
+        if (promise) {
+            await promise;
         }
     }
 
diff --git a/test/test_discordbot.ts b/test/test_discordbot.ts
index 4dd1ad8..4117d25 100644
--- a/test/test_discordbot.ts
+++ b/test/test_discordbot.ts
@@ -25,6 +25,7 @@ import { DiscordBot } from "../src/bot";
 import { MockDiscordClient } from "./mocks/discordclient";
 import { MockMessage } from "./mocks/message";
 import { Util } from "../src/util";
+import { MockChannel } from "./mocks/channel";
 
 // we are a test file and thus need those
 /* tslint:disable:no-unused-expression max-file-line-count no-any */
@@ -415,6 +416,44 @@ describe("DiscordBot", () => {
             assert.equal(expected, ITERATIONS);
         });
     });
+    describe("locks", () => {
+        it("should lock and unlock a channel", async () => {
+            const bot = new modDiscordBot.DiscordBot(
+                "",
+                config,
+                mockBridge,
+                {},
+            ) as DiscordBot;
+            const chan = new MockChannel("123") as any;
+            const t = Date.now();
+            await bot.lockChannel(chan);
+            await bot.waitUnlock(chan);
+            const diff = Date.now() - t;
+            expect(diff).to.be.greaterThan(config.limits.discordSendDelay - 1);
+        });
+        it("should lock and unlock a channel early, if unlocked", async () => {
+            const bot = new modDiscordBot.DiscordBot(
+                "",
+                { 
+                    limits: {
+                        discordSendDelay: 500,
+                    },
+                    bridge: {
+                        domain: "localhost",
+                    }
+                },
+                mockBridge,
+                {},
+            ) as DiscordBot;
+            const chan = new MockChannel("123") as any;
+            setTimeout(() => bot.unlockChannel(chan), 100);
+            const t = Date.now();
+            bot.lockChannel(chan);
+            await bot.waitUnlock(chan);
+            const diff = Date.now() - t;
+            expect(diff).to.be.lessThan(110);
+        });
+    });
   // });
     // describe("ProcessMatrixMsgEvent()", () => {
     //
-- 
GitLab