Skip to content

Backport Kafka dispose deadlock fix to master - #120

Merged
christopherread merged 1 commit into
masterfrom
claude/trusting-curran-ed495b
Oct 25, 2026
Merged

christopherread merged 1 commit into
masterfrom
claude/trusting-curran-ed495b

Conversation

@christopherread

Copy link
Copy Markdown
Owner

Cause

Obvs.Kafka/MessageSource.cs called observer.OnNext on its consume thread, which is the task started with Task.Run in Messages. Disposing the subscription cancels the token and then calls task.Wait() on that same task. So a subscriber that disposed the bus, or its subscription, from inside a message callback waited on itself forever.

Fix

Messages now ends with .ObserveOn(TaskPoolScheduler.Default), so messages reach subscribers on the task pool, still in order. This is the same fix as v7 PR #119. The RabbitMQ version of this bug was fixed the same way in #113.

Test (red/green)

New Explicit test TestMessageSendAndReceive.ShouldNotDeadlockWhenASubscriberDisposesItsSubscription. It subscribes, publishes one message and disposes the subscription from inside the callback. It then checks two things, each with a 10 s timeout: that the message arrived, and that the dispose finished.

  • Before the fix (local Kafka 4.3.1): the test fails with TimeoutException on the second assert. The message arrived but the dispose never returned.
  • After the fix: all 3 Kafka Explicit tests pass on 3 runs in a row.
  • Clean Release build has 0 warnings and 0 errors. Unit tests (--filter-not-trait "Category=Explicit"): 130 passed, 0 failed. Kafka, RabbitMQ and NATS have only Explicit tests, so they report "zero tests ran" under that filter.

🤖 Generated with Claude Code

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>
@christopherread christopherread self-assigned this Oct 25, 2026
@christopherread
christopherread merged commit b0b6856 into master Oct 25, 2026
1 check passed
@christopherread
christopherread deleted the claude/trusting-curran-ed495b branch October 25, 2026 21:08
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>
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.

1 participant