Skip to content

Commit 43e6a5f

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 43e6a5f

File tree

2 files changed

+11
-11
lines changed

2 files changed

+11
-11
lines changed

src/Foundatio.AWS/Messaging/SQSMessageBus.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,16 +181,16 @@ 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;
184+
// Note: Caller (EnsureTopicSubscriptionAsync or PublishAsync) should already hold the lock
185+
// or handle synchronization. We use double-check pattern with ConcurrentDictionary for thread safety.
189186

190-
arn = await CreateTopicImplAsync(topicName, cancellationToken).AnyContext();
191-
_topicArns[topicName] = arn;
187+
// Double-check after potential concurrent access
188+
if (_topicArns.TryGetValue(topicName, out arn))
192189
return arn;
193-
}
190+
191+
arn = await CreateTopicImplAsync(topicName, cancellationToken).AnyContext();
192+
_topicArns.TryAdd(topicName, arn);
193+
return arn;
194194
}
195195

196196
/// <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)