Skip to content

Commit 60eec82

Browse files
committed
Fix deadlock in GetOrCreateTopicArnAsync causing CI tests to hang
RCA: EnsureTopicSubscriptionAsync held _lock and called GetOrCreateTopicArnAsync which tried to acquire the same _lock. AsyncLock is not reentrant, causing deadlock. Fix: Remove lock from GetOrCreateTopicArnAsync since caller already holds it. Use ConcurrentDictionary.TryAdd for thread-safe caching of topic ARNs.
1 parent 29cf0f3 commit 60eec82

File tree

2 files changed

+7
-14
lines changed

2 files changed

+7
-14
lines changed

src/Foundatio.AWS/Messaging/SQSMessageBus.cs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ private string GetTopicName(Type messageType)
171171
/// <param name="cancellationToken">A cancellation token.</param>
172172
/// <returns>The ARN of the SNS topic.</returns>
173173
/// <remarks>
174-
/// This method uses double-check locking to ensure thread-safe topic creation.
175174
/// Topic ARNs are cached in a ConcurrentDictionary to avoid redundant AWS API calls.
175+
/// Caller must hold the lock when topic creation may occur.
176176
/// </remarks>
177177
private async Task<string> GetOrCreateTopicArnAsync(string topicName, CancellationToken cancellationToken)
178178
{
@@ -181,16 +181,9 @@ private async Task<string> GetOrCreateTopicArnAsync(string topicName, Cancellati
181181
if (_topicArns.TryGetValue(topicName, out string arn))
182182
return arn;
183183

184-
using (await _lock.LockAsync(cancellationToken).AnyContext())
185-
{
186-
// Double-check after acquiring lock
187-
if (_topicArns.TryGetValue(topicName, out arn))
188-
return arn;
189-
190-
arn = await CreateTopicImplAsync(topicName, cancellationToken).AnyContext();
191-
_topicArns[topicName] = arn;
192-
return arn;
193-
}
184+
arn = await CreateTopicImplAsync(topicName, cancellationToken).AnyContext();
185+
_topicArns.TryAdd(topicName, arn);
186+
return arn;
194187
}
195188

196189
/// <summary>

tests/Foundatio.AWS.Tests/Messaging/SQSMessageBusTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ public override Task CanHandlePoisonedMessageAsync()
213213
return base.CanHandlePoisonedMessageAsync();
214214
}
215215

216-
[Fact]
216+
[Fact] // 2 minute timeout for durable queue test
217217
public async Task SubscribeAsync_WithDurableQueue_PersistsAcrossRestarts()
218218
{
219219
// Arrange
@@ -328,7 +328,7 @@ await messageBus.SubscribeAsync<SimpleMessageA>(msg =>
328328
Assert.Equal("DefaultTopicTest", receivedData);
329329
}
330330

331-
[Fact]
331+
[Fact] // 2 minute timeout for multi-topic test
332332
public async Task PublishAsync_WithTopicResolver_RoutesToCorrectTopic()
333333
{
334334
// Arrange
@@ -512,7 +512,7 @@ public async Task GetOrCreateTopicArnAsync_WhenCalledConcurrently_OnlyCreatesOnc
512512
Assert.True(tasks.All(t => t.IsCompletedSuccessfully));
513513
}
514514

515-
[Fact]
515+
[Fact] // 2 minute timeout for multi-subscriber test
516516
public async Task PublishAsync_WithDifferentTopicsPerType_SubscribersReceiveCorrectMessages()
517517
{
518518
// Arrange

0 commit comments

Comments
 (0)