[V3 Tunnel utils] Better handling of tunnel instances (#1538)

* Better handling of tunnel instances

* docs

docs/framework_utils.rst

@@ -27,3 +27,9 @@ V2 Data Conversion
 
 .. automodule:: redbot.core.utils.data_converter
     :members: DataConverter
+
+Tunnel
+======
+
+.. automodule:: redbot.core.utils.tunnel
+    :members: Tunnel

redbot/cogs/reports/reports.py

@@ -62,10 +62,6 @@ class Reports:
             x['tun'] for x in self.tunnel_store.values()
         ]
 
-    def __unload(self):
-        for tun in self.tunnels:
-            tun.close()
-
     @checks.admin_or_permissions(manage_guild=True)
     @commands.guild_only()
     @commands.group(name="reportset")

redbot/core/utils/tunnel.py

@@ -3,8 +3,9 @@ from datetime import datetime
 from redbot.core.utils.chat_formatting import pagify
 import io
 import sys
+import weakref
 
-_instances = {}
+_instances = weakref.WeakValueDictionary({})
 
 
 class TunnelMeta(type):
@@ -18,22 +19,35 @@ class TunnelMeta(type):
             (kwargs.get('sender'), kwargs.get('origin')),
             kwargs.get('recipient')
         )
-        if not (
-            any(
-                lockout_tuple[0] == x[0]
-                for x in _instances.keys()
-            ) or any(
-                lockout_tuple[1] == x[1]
-                for x in _instances.keys()
-            )
-        ):
-            _instances[lockout_tuple] = super(
-                TunnelMeta, cls).__call__(*args, **kwargs)
+
+        if lockout_tuple in _instances:
             return _instances[lockout_tuple]
-        elif lockout_tuple in _instances:
-            return _instances[lockout_tuple]
-        else:
-            return None
+
+        # this is needed because weakvalue dicts can
+        # change size without warning if an object is discarded
+        # it can raise a runtime error, so ..
+        while True:
+            try:
+                if not (
+                    any(
+                        lockout_tuple[0] == x[0]
+                        for x in _instances.keys()
+                    ) or any(
+                        lockout_tuple[1] == x[1]
+                        for x in _instances.keys()
+                    )
+                ):
+                    # if this isn't temporarily stored, the weakref dict
+                    # will discard this before the return statement,
+                    # causing a key error
+                    temp = super(TunnelMeta, cls).__call__(*args, **kwargs)
+                    _instances[lockout_tuple] = temp
+                    return temp
+            except:  # NOQA: E722
+                # Am I really supposed to except a runtime error flake >.>
+                continue
+            else:
+                return None
 
 
 class Tunnel(metaclass=TunnelMeta):
@@ -41,9 +55,18 @@ class Tunnel(metaclass=TunnelMeta):
     A tunnel interface for messages
 
     This will return None on init if the destination
-    or source + origin pair is already in use
+    or source + origin pair is already in use, or the
+    existing tunnel object if one exists for the designated
+    parameters
 
-    You should close tunnels when done with them
+    Attributes
+    ----------
+    sender: `discord.Member`
+        The person who opened the tunnel
+    origin: `discord.TextChannel`
+        The channel in which it was opened
+    recipient: `discord.User`
+        The user on the other end of the tunnel
     """
 
     def __init__(self, *,
@@ -55,13 +78,6 @@ class Tunnel(metaclass=TunnelMeta):
         self.recipient = recipient
         self.last_interaction = datetime.utcnow()
 
-    def __del__(self):
-        lockout_tuple = ((self.sender, self.origin), self.recipient)
-        _instances.pop(lockout_tuple, None)
-
-    def close(self):
-        self.__del__()
-
     async def react_close(self, *, uid: int, message: str):
         send_to = self.origin if uid == self.sender.id else self.sender
         closer = next(filter(
@@ -69,20 +85,46 @@ class Tunnel(metaclass=TunnelMeta):
         await send_to.send(
             message.format(closer=closer)
         )
-        self.close()
 
     @property
     def members(self):
-        return (self.sender, self.recipient)
+        return self.sender, self.recipient
 
     @property
     def minutes_since(self):
-        return (self.last_interaction - datetime.utcnow()).minutes
+        return int((self.last_interaction - datetime.utcnow()).seconds / 60)
 
     async def communicate(self, *,
                           message: discord.Message,
                           topic: str=None,
                           skip_message_content: bool=False):
+        """
+        Forwards a message.
+
+        Parameters
+        ----------
+        message : `discord.Message`
+            The message to forward
+        topic : `str`
+            A string to prepend
+        skip_message_content : `bool`
+            If this flag is set, only the topic will be sent
+
+        Returns
+        -------
+        `int`, `int`
+            a pair of ints matching the ids of the
+            message which was forwarded
+            and the last message the bot sent to do that.
+            useful if waiting for reactions.
+
+        Raises
+        ------
+        discord.Forbidden
+            This should only happen if the user's DMs are disabled
+            the bot can't upload at the origin channel
+            or can't add reactions there.
+        """
         if message.channel == self.origin \
                 and message.author == self.sender:
             send_to = self.recipient