Updates to address comments

Author: aidbutlr

CHANGELOG.md

@@ -13,6 +13,8 @@
 - **Missing Server and Gateway Deletes** - `DELETE /servers/{id}` and `DELETE /gateways/{id}` now return `404 Not Found` when the target no longer exists, instead of incorrectly returning `403 Forbidden`.
 - **Custom Auth Headers on Tools** ([#5314](https://github.com/IBM/mcp-context-forge/pull/5314), [#5201](https://github.com/IBM/mcp-context-forge/issues/5201)) - `POST /tools` and `PUT /tools/{tool_id}` now persist the `auth_headers` array instead of silently storing `auth_value: null`. Invalid header keys/values are rejected with a 422 rather than an unhandled 500.
 
+- **Redis Connection Leak** ([#5711](https://github.com/IBM/mcp-context-forge/pull/5711)) - The `mcpgateway/plugins/__init__.py` had a connection leak in `_plugin_invalidation_listener`. The pubsub client is unsubscribed but the connection was never closed, eventually leading to connection pool exhaustion.
+
 ### Changed
 
 - **Stricter `authheaders` Key Validation (Gateways, Tools, A2A Agents)** ([#5314](https://github.com/IBM/mcp-context-forge/pull/5314)) - Header-key validation is now shared across all create/update schemas and the admin form. Keys with embedded whitespace (e.g. `X Api Key`) were previously accepted and stored as invalid HTTP header names that failed at invocation time; they are now rejected with a 422 at config time, and surrounding whitespace is trimmed before storage. Gateway or A2A configs relying on the old behavior will need their header keys corrected on the next update.

mcpgateway/plugins/__init__.py

@@ -528,10 +528,13 @@ async def _plugin_invalidation_listener() -> None:
             backoff = min(max_backoff, backoff * 2)
         finally:
             # Always release the pubsub connection back to the pool.
-            # Using wait_for shields aclose() from an in-flight CancelledError:
+            # wait_for bounds aclose() so a stalled redis-py connection cannot
+            # block the finally clause indefinitely.
             if pubsub is not None:
                 try:
                     await asyncio.wait_for(pubsub.aclose(), timeout=2.0)
+                except asyncio.TimeoutError:
+                    _logger.debug("Plugin invalidation listener: pubsub aclose timed out")
                 except Exception as exc:
                     _logger.error("Plugin invalidation listener: pubsub aclose failed (%s)", exc)
 

tests/unit/mcpgateway/plugins/test_plugin_runtime_management.py

@@ -1816,6 +1816,56 @@ async def _fake_sleep(_delay):
         # aclose() must have been awaited once before the backoff sleep.
         pubsub.aclose.assert_awaited_once()
 
+    @pytest.mark.asyncio
+    async def test_pubsub_aclose_timeout_guard_prevents_hang(self, monkeypatch):
+        """``wait_for(aclose(), timeout=2.0)`` lets the listener exit even when aclose() hangs.
+
+        Regression guard for the timeout introduced to bound a hanging aclose():
+        if the ``timeout=2.0`` parameter were removed (or the wait_for dropped),
+        the finally block would suspend indefinitely and the listener task would
+        never return.
+
+        The test replaces aclose() with a coroutine that sleeps for 999 s and
+        asserts the entire listener finishes well inside that window.
+        """
+        import time
+
+        import mcpgateway.plugins as framework
+
+        async def _fake_handle(msg):
+            raise asyncio.CancelledError()
+
+        monkeypatch.setattr(framework, "_handle_invalidation_message", _fake_handle)
+
+        async def _hanging_aclose():
+            await asyncio.sleep(999)
+
+        pubsub = MagicMock()
+        pubsub.subscribe = AsyncMock()
+        pubsub.aclose = _hanging_aclose  # not an AsyncMock — returns a real coroutine
+
+        class _Listen:
+            def __aiter__(self):
+                return self
+
+            async def __anext__(self):
+                return {"type": "message", "data": "{}"}
+
+        pubsub.listen = MagicMock(return_value=_Listen())
+
+        client = MagicMock()
+        client.pubsub = MagicMock(return_value=pubsub)
+        monkeypatch.setattr(framework, "_redis", AsyncMock(return_value=client))
+
+        start = time.monotonic()
+        await framework._plugin_invalidation_listener()
+        elapsed = time.monotonic() - start
+
+        # The listener must complete well before aclose()'s 999 s sleep expires.
+        # A generous upper bound of 5 s covers slow CI environments while still
+        # being far shorter than the hang duration.
+        assert elapsed < 5.0, f"Listener took {elapsed:.2f}s — timeout guard may be missing"
+
 
 # ---------------------------------------------------------------------------
 # HMAC signing and verification tests