fix (rooms): Harden against race conditions.

Add locking to room init, turnover, and cancellation.
Add cleanup of nonexistent members in slot init.
Fix an issue where members were being charged for cancelling rooms.

bot/modules/accountability/TimeSlot.py

@@ -37,7 +37,7 @@ class SlotMember:
 
     @property
     def member(self):
-        return self.guild.get_member(self.data.userid)
+        return self.guild.get_member(self.userid)
 
     @property
     def has_attended(self):
@@ -254,6 +254,11 @@ class TimeSlot:
         Adds the TimeSlot to cache.
         Returns the (channelid, messageid).
         """
+        # Cleanup any non-existent members
+        for memid, mem in list(self.members.items()):
+            if not mem.data or not mem.member:
+                self.members.pop(memid)
+
         # Calculate overwrites
         overwrites = {
             mem.member: self._member_overwrite

bot/modules/accountability/commands.py

@@ -11,8 +11,9 @@ from data.conditions import GEQ
 from .module import module
 from .lib import utc_now
 from .tracker import AccountabilityGuild as AGuild
+from .tracker import room_lock
 from .TimeSlot import SlotMember
-from .data import accountability_members, accountability_member_info, accountability_open_slots, accountability_rooms
+from .data import accountability_members, accountability_member_info, accountability_rooms
 
 
 @module.cmd(
@@ -104,29 +105,30 @@ async def cmd_rooms(ctx):
         cost = len(to_cancel) * ctx.guild_settings.accountability_price.value
 
         slotids = [row['slotid'] for row in to_cancel]
-        accountability_members.delete_where(
-            userid=ctx.author.id,
-            slotid=slotids
-        )
+        async with room_lock:
+            accountability_members.delete_where(
+                userid=ctx.author.id,
+                slotid=slotids
+            )
 
-        # Handle case where the slot has already opened
-        for row in to_cancel:
-            aguild = AGuild.cache.get(row['guildid'], None)
-            if aguild:
-                if aguild.upcoming_slot and aguild.upcoming_slot.data and (aguild.upcoming_slot.data.slotid in slotids):
-                    aguild.upcoming_slot.members.pop(ctx.author.id, None)
-                    if aguild.upcoming_slot.channel:
-                        try:
-                            await aguild.upcoming_slot.channel.set_permissions(
-                                ctx.author,
-                                overwrite=None
-                            )
-                        except discord.HTTPException:
-                            pass
-                    await aguild.upcoming_slot.update_status()
-                    break
+            # Handle case where the slot has already opened
+            for row in to_cancel:
+                aguild = AGuild.cache.get(row['guildid'], None)
+                if aguild and aguild.upcoming_slot and aguild.upcoming_slot.data:
+                    if aguild.upcoming_slot.data.slotid in slotids:
+                        aguild.upcoming_slot.members.pop(ctx.author.id, None)
+                        if aguild.upcoming_slot.channel:
+                            try:
+                                await aguild.upcoming_slot.channel.set_permissions(
+                                    ctx.author,
+                                    overwrite=None
+                                )
+                            except discord.HTTPException:
+                                pass
+                        await aguild.upcoming_slot.update_status()
+                        break
 
-        ctx.alion.addCoins(-cost)
+        ctx.alion.addCoins(cost)
         await ctx.embed_reply(
             "Successfully canceled your bookings."
         )

bot/modules/accountability/tracker.py

@@ -19,6 +19,19 @@ from .module import module
 
 
 voice_ignore_lock = asyncio.Lock()
+room_lock = asyncio.Lock()
+
+
+def locker(lock):
+    """
+    Function decorator to wrap the function in a provided Lock
+    """
+    def decorator(func):
+        async def wrapped(*args, **kwargs):
+            async with lock:
+                return await func(*args, **kwargs)
+        return wrapped
+    return decorator
 
 
 class AccountabilityGuild:
@@ -184,7 +197,7 @@ async def turnover():
             )
             for slot in current_slots
             for mem in slot.members.values()
-            if mem.member.voice and mem.member.voice.channel != slot.channel
+            if mem.data and mem.member and mem.member.voice and mem.member.voice.channel != slot.channel
         )
         # We return exceptions here to ignore any permission issues that occur with moving members.
         # It's also possible (likely) that members will move while we are moving other members
@@ -279,7 +292,8 @@ async def _accountability_loop():
             next_time = next_time + datetime.timedelta(minutes=5)
             # Open next sessions
             try:
-                await open_next(next_time)
+                async with room_lock:
+                    await open_next(next_time)
             except Exception:
                 # Unknown exception. Catch it so the loop doesn't die.
                 client.log(
@@ -293,7 +307,8 @@ async def _accountability_loop():
         elif next_time.minute == 0:
             # Start new sessions
             try:
-                await turnover()
+                async with room_lock:
+                    await turnover()
             except Exception:
                 # Unknown exception. Catch it so the loop doesn't die.
                 client.log(