Skip to content

Commit eee12bf

Browse files
committed
fix: strengthen concurrency test assertions and fix thread-pool deadlock risk
- Replace Barrier.SignalAndWait (blocking) with SemaphoreSlim (async-safe) in WebhookDeliveryConcurrencyTests and BoardPresenceConcurrencyTests to prevent thread-pool starvation deadlocks under CI - Fix cross-user isolation test race: separate board creation from verification so the check runs against the complete set of board IDs - Assert 429 status code explicitly in ThrottledRequests test instead of silently passing when rate limiting is broken - Strengthen ProcessNext_TwoWorkersTwoItems to assert at least one success and valid status codes (not just "no 500s") - Assert exactly 1 card in DoubleExecute test (was 0-1, hiding data loss) - Assert exactly 1 proposal per batch item (was <=1, hiding data loss)
1 parent 6d5796b commit eee12bf

6 files changed

Lines changed: 72 additions & 43 deletions

File tree

backend/tests/Taskdeck.Api.Tests/Concurrency/BoardPresenceConcurrencyTests.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,23 @@ public async Task RapidJoinLeave_EventuallyConsistent()
9595
await SignalRTestHelper.WaitForEventsAsync(observerEvents, 1);
9696
observerEvents.Clear();
9797

98-
// All users join simultaneously via Barrier
98+
// All users join simultaneously via SemaphoreSlim (async-safe,
99+
// unlike Barrier.SignalAndWait which blocks thread-pool threads)
99100
var connections = new List<HubConnection>();
100101
try
101102
{
102-
using var joinBarrier = new Barrier(connectionCount + 1);
103+
using var joinBarrier = new SemaphoreSlim(0, connectionCount);
103104
var joinTasks = users.Select(async user =>
104105
{
105106
var conn = SignalRTestHelper.CreateBoardsHubConnection(_factory, user.Token);
106107
conn.On<BoardPresenceSnapshot>("boardPresence", _ => { });
107108
await conn.StartAsync();
108109
lock (connections) { connections.Add(conn); }
109-
joinBarrier.SignalAndWait(TimeSpan.FromSeconds(10));
110+
await joinBarrier.WaitAsync(TimeSpan.FromSeconds(10));
110111
await conn.InvokeAsync("JoinBoard", board.Id);
111112
}).ToArray();
112113

113-
joinBarrier.SignalAndWait(TimeSpan.FromSeconds(10));
114+
joinBarrier.Release(connectionCount);
114115
await Task.WhenAll(joinTasks);
115116

116117
// Wait for all joins to settle
@@ -122,14 +123,14 @@ public async Task RapidJoinLeave_EventuallyConsistent()
122123
// First half leave rapidly
123124
observerEvents.Clear();
124125
var leavingCount = connectionCount / 2;
125-
using var leaveBarrier = new Barrier(leavingCount + 1);
126+
using var leaveBarrier = new SemaphoreSlim(0, leavingCount);
126127
var leaveTasks = connections.Take(leavingCount).Select(async conn =>
127128
{
128-
leaveBarrier.SignalAndWait(TimeSpan.FromSeconds(10));
129+
await leaveBarrier.WaitAsync(TimeSpan.FromSeconds(10));
129130
await conn.InvokeAsync("LeaveBoard", board.Id);
130131
}).ToArray();
131132

132-
leaveBarrier.SignalAndWait(TimeSpan.FromSeconds(10));
133+
leaveBarrier.Release(leavingCount);
133134
await Task.WhenAll(leaveTasks);
134135

135136
// Wait for leaves to settle

backend/tests/Taskdeck.Api.Tests/Concurrency/CrossUserIsolationStressTests.cs

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,14 @@ public CrossUserIsolationStressTests(TestWebApplicationFactory factory)
3535
public async Task ConcurrentBoardCreation_NoCrossUserContamination()
3636
{
3737
const int userCount = 5;
38-
var userBoards = new ConcurrentDictionary<string, Guid>();
38+
var userBoards = new ConcurrentDictionary<string, (Guid BoardId, HttpClient Client)>();
3939
var errors = new ConcurrentBag<string>();
4040

41+
// Phase 1: All users create boards concurrently
4142
using var barrier = new SemaphoreSlim(0, userCount);
4243
var tasks = Enumerable.Range(0, userCount).Select(async i =>
4344
{
44-
using var client = _factory.CreateClient();
45+
var client = _factory.CreateClient();
4546
var user = await ApiTestHarness.AuthenticateAsync(client, $"isolation-{i}");
4647
await barrier.WaitAsync();
4748

@@ -53,29 +54,41 @@ public async Task ConcurrentBoardCreation_NoCrossUserContamination()
5354
if (resp.StatusCode != HttpStatusCode.Created)
5455
{
5556
errors.Add($"User {i} got {resp.StatusCode}");
57+
client.Dispose();
5658
return;
5759
}
5860

5961
var board = await resp.Content.ReadFromJsonAsync<BoardDto>();
60-
userBoards[user.Username] = board!.Id;
61-
62-
// Verify user only sees their own board
63-
var listResp = await client.GetAsync("/api/boards");
64-
var boards = await listResp.Content.ReadFromJsonAsync<List<BoardDto>>();
65-
var otherUserBoards = boards!.Where(b =>
66-
userBoards.Any(kv => kv.Key != user.Username && kv.Value == b.Id));
67-
if (otherUserBoards.Any())
68-
{
69-
errors.Add($"User {user.Username} can see another user's board");
70-
}
62+
userBoards[user.Username] = (board!.Id, client);
7163
}).ToArray();
7264

7365
barrier.Release(userCount);
7466
await Task.WhenAll(tasks);
7567

76-
errors.Should().BeEmpty("no cross-user board contamination should occur");
77-
userBoards.Should().HaveCount(userCount,
78-
"all users should have created their boards successfully");
68+
try
69+
{
70+
errors.Should().BeEmpty("all users should create boards successfully");
71+
userBoards.Should().HaveCount(userCount,
72+
"all users should have created their boards successfully");
73+
74+
// Phase 2: After all boards exist, verify isolation with the
75+
// complete set of known board IDs (no race against concurrent inserts)
76+
var allBoardIds = userBoards.Values.Select(v => v.BoardId).ToHashSet();
77+
foreach (var (username, (boardId, client)) in userBoards)
78+
{
79+
var listResp = await client.GetAsync("/api/boards");
80+
var boards = await listResp.Content.ReadFromJsonAsync<List<BoardDto>>();
81+
var visibleOtherBoards = boards!.Where(b =>
82+
allBoardIds.Contains(b.Id) && b.Id != boardId).ToList();
83+
visibleOtherBoards.Should().BeEmpty(
84+
$"user {username} should not see other users' boards");
85+
}
86+
}
87+
finally
88+
{
89+
foreach (var (_, (_, client)) in userBoards)
90+
client.Dispose();
91+
}
7992
}
8093

8194
/// <summary>

backend/tests/Taskdeck.Api.Tests/Concurrency/ProposalApprovalRaceTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,12 +322,12 @@ public async Task DoubleExecute_NoDuplicateSideEffects()
322322
proposal!.Status.Should().Be(ProposalStatus.Applied,
323323
"proposal should be in Applied state after execution");
324324

325-
// Verify at most one card was created (not duplicated)
325+
// Verify exactly one card was created (not duplicated, not lost)
326326
var cardsResp = await client.GetAsync($"/api/boards/{board.Id}/cards");
327327
cardsResp.StatusCode.Should().Be(HttpStatusCode.OK);
328328
var cards = await cardsResp.Content.ReadFromJsonAsync<List<CardDto>>();
329329
var matchingCards = cards!.Count(c => c.Title.Contains("Double execute item"));
330-
matchingCards.Should().BeInRange(0, 1,
331-
"double execute should not create duplicate cards");
330+
matchingCards.Should().Be(1,
331+
"double execute should create exactly one card (no duplicates, no data loss)");
332332
}
333333
}

backend/tests/Taskdeck.Api.Tests/Concurrency/QueueClaimRaceTests.cs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,12 +230,12 @@ public async Task CaptureTriage_BatchConcurrentWorkers_NoItemProcessedTwice()
230230

231231
proposals.Should().NotBeNull();
232232

233-
// Each capture item should have at most one proposal
233+
// Each capture item should have exactly one proposal (no duplicates, no data loss)
234234
foreach (var captureId in captureIds)
235235
{
236236
var matching = proposals!.Count(p => p.SourceReferenceId == captureId.ToString());
237-
matching.Should().BeLessOrEqualTo(1,
238-
$"capture item {captureId} should have at most one proposal (no duplicate processing)");
237+
matching.Should().Be(1,
238+
$"capture item {captureId} should have exactly one proposal (no duplicate processing, no data loss)");
239239
}
240240
}
241241

@@ -278,8 +278,21 @@ public async Task ProcessNext_TwoWorkersTwoItems_EachClaimsDifferentItem()
278278
barrier.Release(2);
279279
await Task.WhenAll(workerTasks);
280280

281+
var responses = responseData.ToList();
282+
281283
// No 500 errors
282-
responseData.Should().NotContain(r => r.Status == HttpStatusCode.InternalServerError,
284+
responses.Should().NotContain(r => r.Status == HttpStatusCode.InternalServerError,
283285
"no internal server errors during concurrent processing");
286+
287+
// At least one worker should succeed (with 2 items, ideally both succeed)
288+
var successResponses = responses.Where(r => r.Status == HttpStatusCode.OK).ToList();
289+
successResponses.Should().NotBeEmpty(
290+
"at least one worker should successfully claim an item");
291+
292+
// All responses should be well-formed (OK or 404, not unexpected errors)
293+
responses.Should().OnlyContain(
294+
r => r.Status == HttpStatusCode.OK || r.Status == HttpStatusCode.NotFound
295+
|| r.Status == HttpStatusCode.BadRequest,
296+
"workers should only get OK, 404, or 400 -- not unexpected errors");
284297
}
285298
}

backend/tests/Taskdeck.Api.Tests/Concurrency/RateLimitingConcurrencyTests.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,12 @@ public async Task ThrottledRequests_IncludeRetryAfterHeader()
145145
"/api/auth/login",
146146
new LoginDto("retry-header-user-2", "wrong-pass"));
147147

148-
if (second.StatusCode == (HttpStatusCode)429)
149-
{
150-
// Retry-After header should be present on 429 responses
151-
second.Headers.Contains("Retry-After").Should().BeTrue(
152-
"429 responses should include a Retry-After header");
153-
}
148+
// Assert the rate limiter actually kicks in before checking headers
149+
second.StatusCode.Should().Be((HttpStatusCode)429,
150+
"the second request should be throttled (permit limit is 1)");
151+
152+
// Retry-After header should be present on 429 responses
153+
second.Headers.Contains("Retry-After").Should().BeTrue(
154+
"429 responses should include a Retry-After header");
154155
}
155156
}

backend/tests/Taskdeck.Api.Tests/Concurrency/WebhookDeliveryConcurrencyTests.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,23 +65,24 @@ public async Task ConcurrentBoardMutations_EachCreatesDeliveryRecord()
6565
.ReadFromJsonAsync<OutboundWebhookSubscriptionSecretDto>();
6666
webhookSub.Should().NotBeNull();
6767

68-
// Create multiple cards concurrently using Barrier
69-
using var barrier = new Barrier(mutationCount + 1);
68+
// Create multiple cards concurrently using SemaphoreSlim (async-safe,
69+
// unlike Barrier.SignalAndWait which blocks thread-pool threads)
70+
using var barrier = new SemaphoreSlim(0, mutationCount);
7071
var statusCodes = new ConcurrentBag<HttpStatusCode>();
7172

7273
var mutationTasks = Enumerable.Range(0, mutationCount).Select(async i =>
7374
{
7475
using var raceClient = _factory.CreateClient();
7576
raceClient.DefaultRequestHeaders.Authorization =
7677
client.DefaultRequestHeaders.Authorization;
77-
barrier.SignalAndWait(TimeSpan.FromSeconds(10));
78+
await barrier.WaitAsync();
7879
var resp = await raceClient.PostAsJsonAsync(
7980
$"/api/boards/{board.Id}/cards",
8081
new CreateCardDto(board.Id, col!.Id, $"Webhook card {i}", null, null, null));
8182
statusCodes.Add(resp.StatusCode);
8283
}).ToArray();
8384

84-
barrier.SignalAndWait(TimeSpan.FromSeconds(10));
85+
barrier.Release(mutationCount);
8586
await Task.WhenAll(mutationTasks);
8687

8788
// All card creations should succeed
@@ -135,15 +136,15 @@ public async Task ConcurrentSubscriptionCreation_AllSucceedWithDistinctIds()
135136
await ApiTestHarness.AuthenticateAsync(client, "webhook-concurrent-sub");
136137
var board = await ApiTestHarness.CreateBoardAsync(client, "webhook-sub-board");
137138

138-
using var barrier = new Barrier(subscriptionCount + 1);
139+
using var barrier = new SemaphoreSlim(0, subscriptionCount);
139140
var results = new ConcurrentBag<(HttpStatusCode Status, OutboundWebhookSubscriptionSecretDto? Sub)>();
140141

141142
var tasks = Enumerable.Range(0, subscriptionCount).Select(async i =>
142143
{
143144
using var raceClient = _factory.CreateClient();
144145
raceClient.DefaultRequestHeaders.Authorization =
145146
client.DefaultRequestHeaders.Authorization;
146-
barrier.SignalAndWait(TimeSpan.FromSeconds(10));
147+
await barrier.WaitAsync();
147148
var resp = await raceClient.PostAsJsonAsync(
148149
$"/api/boards/{board.Id}/webhooks",
149150
new CreateOutboundWebhookSubscriptionDto(
@@ -155,7 +156,7 @@ public async Task ConcurrentSubscriptionCreation_AllSucceedWithDistinctIds()
155156
results.Add((resp.StatusCode, sub));
156157
}).ToArray();
157158

158-
barrier.SignalAndWait(TimeSpan.FromSeconds(10));
159+
barrier.Release(subscriptionCount);
159160
await Task.WhenAll(tasks);
160161

161162
// All should succeed

0 commit comments

Comments
 (0)