Fix relay race condition in duckpond using a lock

Our duckpond suffered from a race condition: If multiple raw reaction
events were received in quick succession and a message had enough ducks
to take it over the duckpond threshold, the message would be relayed
multiple times.

The reason this happened is because the green checkmark emoji that stops
a message from being relayed multiple times is only added after the
message has been relayed. This means that multiple event triggers can
make it past the green checkmark check before any of them has a chance
to add a green checkmark.

The solution was to create a relay lock that needs to be acquired before
checking for the presence of a green checkmark and is only released
after adding a green checkmark. This prevents multiple events from
making it past the sentinel check.

As our Cogs are potentially initialized before the event loop is
created, the lock is load lazily when needed.

Signed-off-by: Sebastiaan Zeeff <[email protected]>

1 files changed, 17 insertions, 7 deletions

diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py
index 84c0c4265..12f4cb7b8 100644
--- a/bot/cogs/duck_pond.py
+++ b/bot/cogs/duck_pond.py
@@ -1,3 +1,4 @@
+import asyncio
 import logging
 from typing import Union
 
@@ -21,6 +22,7 @@ class DuckPond(Cog):
         self.webhook_id = constants.Webhooks.duck_pond
         self.webhook = None
         self.bot.loop.create_task(self.fetch_webhook())
+        self.relay_lock = None
 
     async def fetch_webhook(self) -> None:
         """Fetches the webhook object, so we can post to it."""
@@ -103,8 +105,6 @@ class DuckPond(Cog):
             except discord.HTTPException:
                 log.exception("Failed to send an attachment to the webhook")
 
-        await message.add_reaction("✅")
-
     def _payload_has_duckpond_emoji(self, emoji: discord.PartialEmoji) -> bool:
         """Test if the RawReactionActionEvent payload contains a duckpond emoji."""
         if emoji.is_unicode_emoji():
@@ -145,16 +145,26 @@ class DuckPond(Cog):
         if not self.is_staff(member) or member.bot:
             return
 
-        # Does the message already have a green checkmark?
-        if await self.has_green_checkmark(message):
-            return
-
         # Time to count our ducks!
         duck_count = await self.count_ducks(message)
 
         # If we've got more than the required amount of ducks, send the message to the duck_pond.
         if duck_count >= constants.DuckPond.threshold:
-            await self.relay_message(message)
+            if self.relay_lock is None:
+                # Lazily load the lock to ensure it's created within the
+                # appropriate event loop.
+                self.relay_lock = asyncio.Lock()
+
+            async with self.relay_lock:
+                # check if the message has a checkmark after acquiring the lock
+                if await self.has_green_checkmark(message):
+                    return
+
+                # relay the message
+                await self.relay_message(message)
+
+                # add a green checkmark to indicate that the message was relayed
+                await message.add_reaction("✅")
 
     @Cog.listener()
     async def on_raw_reaction_remove(self, payload: RawReactionActionEvent) -> None: