release upnp ports on shutdown (#4024)

* release upnp ports on shutdown

* lint

* explicitly removes port mapping first to handle possible unclean shutdowns

* catches exception

* moves upnp setup to thread

* lint

* moves upnp logic to a message queue on a worker thread

* lint

chia/server/start_service.py

@@ -15,7 +15,7 @@ except ImportError:
 from chia.rpc.rpc_server import start_rpc_server
 from chia.server.outbound_message import NodeType
 from chia.server.server import ChiaServer
-from chia.server.upnp import upnp_remap_port
+from chia.server.upnp import upnp
 from chia.types.peer_info import PeerInfo
 from chia.util.chia_logging import initialize_logging
 from chia.util.config import load_config, load_config_cli
@@ -127,7 +127,7 @@ class Service:
         await self._node._start(**kwargs)
 
         for port in self._upnp_ports:
-            upnp_remap_port(port)
+            upnp.remap(port)
 
         await self._server.start_server(self._on_connect_callback)
 
@@ -189,6 +189,9 @@ class Service:
 
                 self._rpc_close_task = asyncio.create_task(close_rpc_server())
 
+            for port in self._upnp_ports:
+                upnp.release(port)
+
     async def wait_closed(self) -> None:
         await self._is_stopping.wait()
 

chia/server/upnp.py

@@ -1,4 +1,6 @@
 import logging
+import threading
+from queue import Queue
 
 try:
     import miniupnpc
@@ -9,14 +11,55 @@ except ImportError:
 log = logging.getLogger(__name__)
 
 
-def upnp_remap_port(port) -> None:
-    log.info(f"Attempting to enable UPnP (open up port {port})")
-    try:
-        upnp = miniupnpc.UPnP()
-        upnp.discoverdelay = 30
-        upnp.discover()
-        upnp.selectigd()
-        upnp.addportmapping(port, "TCP", upnp.lanaddr, port, "chia", "")
-        log.info(f"Port {port} opened with UPnP. lanaddr {upnp.lanaddr} external: {upnp.externalipaddress()}")
-    except Exception:
-        log.info("UPnP failed. This is not required to run chia, but it allows incoming connections from other peers.")
+class UPnP:
+    def __init__(self):
+        self.queue = Queue()
+
+        def run():
+            try:
+                self.upnp = miniupnpc.UPnP()
+                self.upnp.discoverdelay = 30
+                self.upnp.discover()
+                self.upnp.selectigd()
+                keep_going = True
+                while keep_going:
+                    msg = self.queue.get()
+                    if msg[0] == "remap":
+                        port = msg[1]
+                        log.info(f"Attempting to enable UPnP (open up port {port})")
+                        self.upnp.deleteportmapping(port, "TCP")
+                        self.upnp.addportmapping(port, "TCP", self.upnp.lanaddr, port, "chia", "")
+                        log.info(
+                            f"Port {port} opened with UPnP. lanaddr {self.upnp.lanaddr} "
+                            f"external: {self.upnp.externalipaddress()}"
+                        )
+                    elif msg[0] == "release":
+                        port = msg[1]
+                        self.upnp.deleteportmapping(port, "TCP")
+                        log.info(f"Port {port} closed with UPnP")
+                    elif msg[0] == "shutdown":
+                        keep_going = False
+            except Exception as e:
+                log.info(
+                    "UPnP failed. This is not required to run chia, it allows incoming connections from other peers."
+                )
+                log.info(e)
+
+        self.thread = threading.Thread(target=run)
+        self.thread.start()
+
+    def remap(self, port):
+        self.queue.put(("remap", port))
+
+    def release(self, port):
+        self.queue.put(("release", port))
+
+    def shutdown(self):
+        self.queue.put(("shutdown",))
+        self.thread.join()
+
+    def __del__(self):
+        self.shutdown()
+
+
+upnp: UPnP = UPnP()