BrighterCommand / BrighterCommand/Brighter
Azure Service Bus producers throw a 409 when several threads race to create the same topic
- Dominant language
- C#
- Stars
- 2.5k
- Forks
- 296
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 21
Description
## Summary
Azure Service Bus producers create their topic/queue lazily on the send path, using an unsynchronised
check-then-act that does not tolerate losing the race. When several threads publish to the same
not-yet-created topic at once, they all observe "does not exist", they all call `CreateTopicAsync`, and
the losers get a 409 that is rethrown rather than absorbed:
```
Azure.Messaging.ServiceBus.ServiceBusException : SubCode=40900. Conflict. You're requesting an
operation that isn't allowed in the resource's current state.
```
(`SubCode=40901`, "Another conflicting operation is in progress", appears from the same race.)
## The offending code
`src/Paramore.Brighter.MessagingGateway.AzureServiceBus/AzureServiceBusTopicMessageProducer.cs:62-89`
— `EnsureChannelExistsAsync`:
```csharp
if (await _administrationClientWrapper.TopicExistsAsync(channelName)) // check
{
TopicCreated = true;
return;
}
...
await _administrationClientWrapper.CreateTopicAsync(channelName); // act
TopicCreated = true;
```
There is no lock, and the `catch` rethrows every exception unconditionally. `TopicCreated` is set only
*after* a successful create, so it cannot serialise concurrent callers either.
`AzureServiceBusQueueMessageProducer.EnsureChannelExistsAsync` (`:62-89`) has the identical shape for
`QueueExistsAsync` / `CreateQueueAsync`.
This runs on the **send** path — `SendWithDelayAsync` → `GetSenderAsync` → `EnsureChannelExistsAsync`
(`AzureServiceBusMessageProducer.cs:248`) — so any concurrent first publish to a new topic can hit it.
## The asymmetry that shows the fix
**Both consumers already handle this correctly.** `AzureServiceBusTopicConsumer.cs:103` and
`AzureServiceBusQueueConsumer.cs:112` both do:
```csharp
catch (ServiceBusException ex)
{
if (ex.Reason == ServiceBusFailureReason.MessagingEntityAlreadyExists)
{
Log.MessageEntityAlreadyExists(s_logger, Topic, _subscriptionName);
_subscriptionCreated = true; // losing the race is success
}
else { ... }
}
```
**Neither producer does** — `grep -rn 'MessagingEntityAlreadyExists' src/Paramore.Brighter.MessagingGateway.AzureServiceBus/`
returns exactly those two consumer sites and nothing else. Losing a creation race is a success, not a
failure: the entity you wanted now exists.
## Observed failure
CI run [`34127954869`, job `101763183832`](https://github.com/BrighterCommand/Brighter/actions/runs/34127954869)
(`azure-ci`): `Total tests: 161 / Passed: 131 / Failed: 30`. Two of those 30 are this defect — both
variants of `When_multiple_threads_try_to_post_a_message_at_the_same_time_should_not_throw_exception`,
Reactor and Proactor. That test does exactly what the name says, which is why it is the one that trips
the race.
The other 28 split into #4309 (26) and #4310 (2).
⚠️ **Do not confuse this with shared-namespace contention between concurrent CI runs.** That also
produces 409s on the same namespace, but this one is a race *inside a single test*, and it is
reproducible: the test posts from multiple threads by design.
## Suggested fix
Absorb `ServiceBusFailureReason.MessagingEntityAlreadyExists` in both producers' `EnsureChannelExistsAsync`
and set `TopicCreated = true`, mirroring what the two consumers already do. Consider also guarding the
check-then-act with a `SemaphoreSlim` so a single process does not send N redundant create calls, though
absorbing the 409 is sufficient for correctness since another process can always win the race.
Found while investigating the Azure Service Bus conformance deferrals in #4240, alongside #4309 and #4310.
Contributor guide
Assessment
This issue has not been assessed yet.