From a454fa59284c65d499f672e846dcc8f7215bc9c0 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Thu, 18 Dec 2025 06:02:03 +0000
Subject: [PATCH 1/2] Initial plan


From 72e57f9ccf5e4d3e925fb7db6b51879402002a89 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Thu, 18 Dec 2025 06:17:52 +0000
Subject: [PATCH 2/2] Fix event handler leak: unregister handlers in
 RpcTargetInfo.DisposeAsync

Co-authored-by: AArnott <3548+AArnott@users.noreply.github.com>
---
 src/StreamJsonRpc/Reflection/RpcTargetInfo.cs |  3 +
 .../TargetObjectEventsTests.cs                | 74 +++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs b/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs
index baf324a2..333b8ec9 100644
--- a/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs
+++ b/src/StreamJsonRpc/Reflection/RpcTargetInfo.cs
@@ -42,6 +42,9 @@ internal RpcTargetInfo(JsonRpc jsonRpc)
 
     public async ValueTask DisposeAsync()
     {
+        // Unregister event handlers first to prevent any events from being raised during disposal.
+        this.UnregisterEventHandlersFromTargetObjects();
+
         List<object>? objectsToDispose;
         lock (this.SyncObject)
         {
diff --git a/test/StreamJsonRpc.Tests/TargetObjectEventsTests.cs b/test/StreamJsonRpc.Tests/TargetObjectEventsTests.cs
index da9d8028..c544bb72 100644
--- a/test/StreamJsonRpc.Tests/TargetObjectEventsTests.cs
+++ b/test/StreamJsonRpc.Tests/TargetObjectEventsTests.cs
@@ -150,6 +150,80 @@ public void GenericServerEventSubscriptionLifetime()
         Assert.Null(this.server.ServerEventWithCustomArgsAccessor);
     }
 
+    /// <summary>
+    /// Verifies that event handlers are unregistered when multiple connections are established and disposed.
+    /// This tests for a memory leak where event handlers would accumulate if not properly unregistered on disposal.
+    /// </summary>
+    [Fact]
+    public void EventHandlersUnregisteredOnMultipleConnectionDisposals()
+    {
+        // Use a shared server object across multiple connections to simulate the real-world scenario
+        var sharedServer = new Server();
+
+        // Verify no handlers initially
+        Assert.Null(sharedServer.ServerEventAccessor);
+        Assert.Null(sharedServer.ServerEventWithCustomArgsAccessor);
+
+        // Create and dispose multiple connections
+        for (int i = 0; i < 3; i++)
+        {
+            var streams = FullDuplexStream.CreateStreams();
+            var rpc = this.CreateJsonRpcWithTargetObject(streams.Item1, sharedServer);
+            rpc.StartListening();
+
+            // Verify handler is registered
+            Assert.NotNull(sharedServer.ServerEventAccessor);
+            Assert.NotNull(sharedServer.ServerEventWithCustomArgsAccessor);
+
+            // Count the number of handlers attached
+            int serverEventHandlerCount = sharedServer.ServerEventAccessor?.GetInvocationList().Length ?? 0;
+            int customArgsEventHandlerCount = sharedServer.ServerEventWithCustomArgsAccessor?.GetInvocationList().Length ?? 0;
+
+            // Should only have one handler per event, not accumulating
+            Assert.Equal(1, serverEventHandlerCount);
+            Assert.Equal(1, customArgsEventHandlerCount);
+
+            // Dispose the connection
+            rpc.Dispose();
+
+            // Verify handlers are unregistered after disposal
+            Assert.Null(sharedServer.ServerEventAccessor);
+            Assert.Null(sharedServer.ServerEventWithCustomArgsAccessor);
+        }
+    }
+
+    /// <summary>
+    /// Verifies that event handlers are unregistered when the stream is closed without explicit disposal.
+    /// This simulates the scenario where a websocket connection drops unexpectedly.
+    /// </summary>
+    [Fact]
+    public async Task EventHandlersUnregisteredWhenStreamClosesUnexpectedly()
+    {
+        var sharedServer = new Server();
+
+        // Verify no handlers initially
+        Assert.Null(sharedServer.ServerEventAccessor);
+
+        var streams = FullDuplexStream.CreateStreams();
+        var serverRpc = this.CreateJsonRpcWithTargetObject(streams.Item1, sharedServer);
+        var clientRpc = new JsonRpc(streams.Item2);
+
+        serverRpc.StartListening();
+        clientRpc.StartListening();
+
+        // Verify handler is registered
+        Assert.NotNull(sharedServer.ServerEventAccessor);
+
+        // Simulate connection drop by closing the stream without disposing JsonRpc
+        streams.Item2.Dispose();
+
+        // Wait for the disconnection to be detected
+        await serverRpc.Completion.WithCancellation(this.TimeoutToken);
+
+        // Verify handlers are unregistered after stream closure
+        Assert.Null(sharedServer.ServerEventAccessor);
+    }
+
     /// <summary>Ensures that JsonRpc only adds one event handler to target objects where events are declared multiple times in an type hierarchy.</summary>
     /// <remarks>This is a regression test for <see href="https://github.com/microsoft/vs-streamjsonrpc/issues/481">this bug</see>.</remarks>
     [Fact]