Skip to content

Conversation

@matteo-prosperi
Copy link
Member

This addresses #1087

Copy link
Member

@AArnott AArnott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Is the theory of the bug then that DisposeAsync was being called concurrently?

@matteo-prosperi
Copy link
Member Author

@AArnott,
the issue was easily reproducible with the sample provided in #1087.

The sample has a loop of the following:

        await using var stream = new NamedPipeClientStream(".", "TestPipe", PipeDirection.InOut, PipeOptions.Asynchronous);
        await stream.ConnectAsync().ConfigureAwait(false);

        using var jsonRpc = JsonRpc.Attach(stream);
        var serverProxy = jsonRpc.Attach<IServer>();

        using var myService = await serverProxy.GetMyService().ConfigureAwait(false);
        await myService.DoSomething().ConfigureAwait(false);

Which results in frequent exceptions on the following line, due to the collection having been modified concurrently with the iteration

foreach (object target in this.localTargetObjectsToDispose)

I verified that the issue doesn't repro anymore after my change.

I believe that the cause of the concurrent change of the collection is RevertAddLocalRpcTarget.Dispose().

@matteo-prosperi matteo-prosperi merged commit 1e0549c into main Feb 25, 2025
6 checks passed
@matteo-prosperi matteo-prosperi deleted the dev/maprospe/fix_proxy_disposal branch February 25, 2025 20:20
@AArnott AArnott added this to the v2.21 milestone Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants