Skip to content

Commit c983773

Browse files
alisan3claude
andcommitted
Return the pooled dispatch context after a successful reply
ReplyAsync returned its pooled DispatchContext only from a catch block, so a reply that succeeded never gave the context back. The pool stayed empty and every reply allocated a fresh context, which is the opposite of what the pool is for. The five sibling operations on the bus all use finally; this one was the exception. Pre-existing, but this branch routes fault replies through ReplyAsync as well, so it sends more traffic through the defect. Dispatch middleware serializes what it needs rather than retaining the context, which the publish and send paths already rely on, so finally is safe here for the same reason it is safe there. finally does not catch, so the exception still reaches the caller as it did when the catch rethrew it. The second test pins that contract by throwing from a dispatch middleware and asserting both the exception and the balanced pool. It passes against the old code too, which is what makes it evidence that only the success path changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1ffbbc3 commit c983773

2 files changed

Lines changed: 135 additions & 2 deletions

File tree

src/Mocha/src/Mocha/Middlewares/DefaultMessageBus.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,9 @@ public async ValueTask ReplyAsync<TResponse>(
243243

244244
await replyEndpoint.ExecuteAsync(context);
245245
}
246-
catch
246+
finally
247247
{
248248
_contextPool.Return(context);
249-
throw;
250249
}
251250
}
252251

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
using Microsoft.Extensions.DependencyInjection;
2+
using Microsoft.Extensions.ObjectPool;
3+
using Mocha.Middlewares;
4+
using Mocha.Transport.InMemory;
5+
6+
namespace Mocha.Tests.Infrastructure;
7+
8+
/// <summary>
9+
/// Every bus operation rents a <see cref="DispatchContext"/> from the pool and has to return it, so
10+
/// that a steady stream of messages does not allocate a context per message.
11+
/// </summary>
12+
public class DispatchContextPoolingTests
13+
{
14+
private static readonly TimeSpan s_timeout = TimeSpan.FromSeconds(10);
15+
16+
[Fact]
17+
public async Task ReplyAsync_Should_ReturnPooledContext_When_ReplySucceeds()
18+
{
19+
// arrange
20+
var pool = new CountingDispatchContextPool();
21+
var services = new ServiceCollection();
22+
services.AddSingleton<ObjectPool<DispatchContext>>(pool);
23+
var builder = services.AddMessageBus();
24+
builder.AddRequestHandler<PoolingRequestHandler>();
25+
builder.AddInMemory();
26+
27+
await using var provider = services.BuildServiceProvider();
28+
var runtime = (MessagingRuntime)provider.GetRequiredService<IMessagingRuntime>();
29+
await runtime.StartAsync(CancellationToken.None);
30+
31+
using var scope = provider.CreateScope();
32+
var bus = scope.ServiceProvider.GetRequiredService<IMessageBus>();
33+
34+
// act - the request rents a context, and the reply rents another on the consumer side
35+
var response = await bus.RequestAsync(new PoolingRequest(), CancellationToken.None);
36+
37+
// the reply is dispatched on the consumer's flow, so let it unwind before counting
38+
var deadline = DateTime.UtcNow + s_timeout;
39+
while (pool.Rented != pool.Returned && DateTime.UtcNow < deadline)
40+
{
41+
await Task.Delay(25, TestContext.Current.CancellationToken);
42+
}
43+
44+
// assert
45+
Assert.NotNull(response);
46+
Assert.Equal(pool.Rented, pool.Returned);
47+
}
48+
49+
[Fact]
50+
public async Task ReplyAsync_Should_PropagateAndReturnPooledContext_When_DispatchThrows()
51+
{
52+
// The pooled context is returned from a finally rather than a catch, so the exception has to
53+
// keep propagating to the caller exactly as it did when the catch rethrew it.
54+
55+
// arrange
56+
var pool = new CountingDispatchContextPool();
57+
var services = new ServiceCollection();
58+
services.AddSingleton<ObjectPool<DispatchContext>>(pool);
59+
var builder = services.AddMessageBus();
60+
builder.AddRequestHandler<PoolingRequestHandler>();
61+
builder.ConfigureMessageBus(b => b.UseDispatch(ThrowOnReplyMiddleware.Create()));
62+
builder.AddInMemory();
63+
64+
await using var provider = services.BuildServiceProvider();
65+
var runtime = (MessagingRuntime)provider.GetRequiredService<IMessagingRuntime>();
66+
await runtime.StartAsync(CancellationToken.None);
67+
68+
using var scope = provider.CreateScope();
69+
var bus = scope.ServiceProvider.GetRequiredService<IMessageBus>();
70+
var replyAddress = runtime.Transports.Single().ReplyReceiveEndpoint!.Source.Address!;
71+
72+
var options = new ReplyOptions
73+
{
74+
Headers = [],
75+
CorrelationId = Guid.NewGuid().ToString(),
76+
ReplyAddress = replyAddress
77+
};
78+
79+
// act & assert - the dispatch failure still surfaces to the caller
80+
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
81+
async () => await bus.ReplyAsync(new PoolingResponse(), options, CancellationToken.None));
82+
83+
Assert.Equal("dispatch failed", ex.Message);
84+
85+
// assert - and the context was still handed back
86+
Assert.Equal(pool.Rented, pool.Returned);
87+
}
88+
89+
private sealed class ThrowOnReplyMiddleware
90+
{
91+
public static DispatchMiddlewareConfiguration Create()
92+
=> new(
93+
static (_, next) => ctx =>
94+
ctx.Headers.Get(MessageHeaders.MessageKind) == MessageKind.Reply
95+
? throw new InvalidOperationException("dispatch failed")
96+
: next(ctx),
97+
"ThrowOnReply");
98+
}
99+
100+
private sealed class CountingDispatchContextPool : ObjectPool<DispatchContext>
101+
{
102+
private readonly DispatchContextPool _inner = new();
103+
private int _rented;
104+
private int _returned;
105+
106+
public int Rented => Volatile.Read(ref _rented);
107+
108+
public int Returned => Volatile.Read(ref _returned);
109+
110+
public override DispatchContext Get()
111+
{
112+
Interlocked.Increment(ref _rented);
113+
return _inner.Get();
114+
}
115+
116+
public override void Return(DispatchContext obj)
117+
{
118+
Interlocked.Increment(ref _returned);
119+
_inner.Return(obj);
120+
}
121+
}
122+
123+
public sealed record PoolingRequest : IEventRequest<PoolingResponse>;
124+
125+
public sealed record PoolingResponse;
126+
127+
public sealed class PoolingRequestHandler : IEventRequestHandler<PoolingRequest, PoolingResponse>
128+
{
129+
public ValueTask<PoolingResponse> HandleAsync(
130+
PoolingRequest request,
131+
CancellationToken cancellationToken)
132+
=> new(new PoolingResponse());
133+
}
134+
}

0 commit comments

Comments
 (0)