Repository navigation
ActiveMQ: deliver messages to subscribers off the session's dispatch thread - #122
Merged
Merged
Conversation
…thread MessageSource passed each message to subscribers synchronously on the NMS session's dispatch thread. Code that ran after awaiting a message could then run on that thread, and deadlock it by disposing the bus: stopping the connection waits for the dispatch thread, forever. NMS skips that wait only for code in the dispatch's own execution context, which an await continuation isn't. Messages are now handed to subscribers on the task pool, in order. TestServiceBus.ShouldNotDeadlockWhenCodeAfterAwaitingAMessageDisposesTheBus hung before this change. Five TestMessageSource tests now wait for delivery instead of expecting it on the listener's thread. CI runs the ActiveMQ broker tests against apache/activemq-classic. Backport of the fix in v7 PR #121. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
christopherread
added a commit
that referenced
this pull request
Oct 25, 2026
* Add v7 'AI native' design doc (#97) * Add v7 'AI native' design doc Proposes how v7 changes configuration, message registration, handlers, testing, errors, observability, contracts, packaging and docs so AI coding agents (and people) can use Obvs correctly first time. Each change links to its tracking issue in the v7 milestone (#84-#95). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Design doc: frame the async APIs as layers over the observable core Section 4 now explains why v7 stays observable-first: handlers, RequestAsync and IAsyncEnumerable are thin adapters over IObservable, Rx stays first class for stream composition, and analyzer OBVS006 catches Subscribe(async). Resolves the open question on Rx: it stays in the core package. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Design doc: describe the bus lifetime under dependency injection Section 1 now covers how the singleton bus is registered and owned, scoped handlers, repeated AddObvs calls, keyed buses, and why one bus per container needs a per-bus instance ID in broker-side names instead of the process ID used by v6. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * NetMQ: run each subscription's receive loop on its own thread (#110) MessageSource started its receive loop with Task.Run. The loop blocks for as long as the subscription lasts, so each subscription held a thread-pool thread, and Subscribe waited until the loop got one. On CI's small runners, the NetMQ tests start a dozen subscriptions at once; the pool added threads slowly, subscribers took over 5 seconds to connect, publishers stopped waiting for them, and the first messages were dropped. The loop now runs as a LongRunning task on a dedicated thread. TestMessageSourceThreading saturates the thread pool and checks that subscribing doesn't wait for it. It failed before this change. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * RabbitMQ: deliver messages to subscribers off the consumer-dispatch thread (#113) MessageSource passed each message to subscribers synchronously on RabbitMQ.Client's consumer-dispatch thread. Code running after a message arrived, including code after awaiting one (the continuation runs inline there), could deadlock it: disposing the bus closes the connection, which waits for that same thread. Messages are still deserialized on the dispatch thread, while the body's buffer is valid, then handed to subscribers on the task pool, in order. TestServiceBus.ShouldNotDeadlockWhenASubscriberDisposesTheBus disposes the bus from a message callback; it timed out before this change. Backport of the fix in v7 PR #112. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * CI: pin the .NET SDK to 10.0.402 (#117) Microsoft's download origin returns 409 for the new SDK 10.0.403 and runtime 10.0.14, so setup-dotnet fails to install 10.0.x. Go back to 10.0.x once that release downloads. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * Kafka: deliver messages to subscribers off the consume thread (#120) MessageSource called observer.OnNext on its consume thread, the task started by Task.Run in Messages. Disposing the subscription cancels the token and then waits for that task, so a subscriber that disposed the bus or its subscription from a message callback waited on itself forever. Messages are now handed to subscribers on the task pool, in order. TestMessageSendAndReceive.ShouldNotDeadlockWhenASubscriberDisposesItsSubscription disposes the subscription from a message callback; the message arrived but the dispose timed out before this change. Backport of the fix in v7 PR #119; the same fix for RabbitMQ was #113. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * ActiveMQ: deliver messages to subscribers off the session's dispatch thread (#122) MessageSource passed each message to subscribers synchronously on the NMS session's dispatch thread. Code that ran after awaiting a message could then run on that thread, and deadlock it by disposing the bus: stopping the connection waits for the dispatch thread, forever. NMS skips that wait only for code in the dispatch's own execution context, which an await continuation isn't. Messages are now handed to subscribers on the task pool, in order. TestServiceBus.ShouldNotDeadlockWhenCodeAfterAwaitingAMessageDisposesTheBus hung before this change. Five TestMessageSource tests now wait for delivery instead of expecting it on the listener's thread. CI runs the ActiveMQ broker tests against apache/activemq-classic. Backport of the fix in v7 PR #121. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport to v6 of the ActiveMQ fix in #121 (v7). The same bug was fixed for RabbitMQ in #113 and for Kafka in #120.
The bug
MessageSourcepassed each message to subscribers synchronously on the NMS session's dispatch thread, from the consumerListenercallback. Code that ran afterawaiting a message could then run on that thread. If that code disposed the bus, it deadlocked:ServiceBus.Dispose→Connection.Stop()→SessionExecutor.Stop()→DedicatedTaskRunner.Shutdown()waits for the dispatch thread it is running on.On v6, disposing the bus directly inside a
Subscribecallback does not hang. NMS skips the wait when the caller is in the dispatch's own execution context (anAsyncLocal<bool>). A continuation after anawaitruns with the awaiting code's execution context, so NMS waits, and the wait never ends. The new test therefore disposes the bus after awaiting a message. Adotnet-stackdump of the hung test process showed the dispatch thread blocked inMonitor.WaitunderDedicatedTaskRunner.Shutdown, called fromConnection.StopinServiceBus.Dispose.The fix
In
MessageSource,.ObserveOn(TaskPoolScheduler.Default)after deserializing and acknowledging, so messages still arrive in order but on the task pool. Messages are still deserialized and acknowledged on the dispatch thread.Tests
TestServiceBus.ShouldNotDeadlockWhenCodeAfterAwaitingAMessageDisposesTheBus(Category=Explicit, needs a broker). It awaits a command and then disposes the bus, and must finish within 10 s. Before the fix it hung: the 10 s timeout never got to report, because disposing the subscription at the end of the test then blocked on the connection's session lock. It passed 3 runs in a row after the fix, against ActiveMQ Classic 6.3.2 run locally.TestBrokerreadsACTIVEMQ_URI, defaulting toactivemq:tcp://localhost:61616. The test project now referencesObvs.Serialization.Json.TestMessageSourcetests assertedOnNextstraight after raising the fake listener event. That assumed delivery on the listener's thread. They now call anAssertDeliveredhelper, which waits up to 5 s for the call and then makes the same assertion. The ActiveMQ unit tests passed 5 runs in a row.apache/activemq-classic:6.2.0service,ACTIVEMQ_URIin the broker test step, and the ActiveMQ Explicit tests in that step's run list. The SDK pin is unchanged.The Release build has 0 warnings, and all 130 unit tests pass (
--filter-not-trait "Category=Explicit").🤖 Generated with Claude Code