CritasWang commented on code in PR #62:
URL:
https://github.com/apache/iotdb-client-csharp/pull/62#discussion_r3688062462
##########
src/Apache.IoTDB/ConcurrentClientQueue.cs:
##########
@@ -70,7 +90,7 @@ public Client Take()
bool timeout = false;
if (ClientQueue.IsEmpty)
{
- timeout = !Monitor.Wait(ClientQueue,
TimeSpan.FromSeconds(Timeout));
+ timeout = !Monitor.Wait(ClientQueue,
TimeSpan.FromMilliseconds(TimeoutInMs));
Review Comment:
Confirmed and fixed in f855e96 — you are right that the original failure
mode was still reachable under contention.
`Take()` now captures the budget once and recomputes the remaining duration
on each wake-up:
```csharp
var budgetMs = TimeoutInMs;
var elapsed = Stopwatch.StartNew();
while (true)
{
if (ClientQueue.TryDequeue(out client)) break;
var remainingMs = budgetMs - (int)elapsed.ElapsedMilliseconds;
if (remainingMs <= 0) break;
Monitor.Wait(ClientQueue, TimeSpan.FromMilliseconds(remainingMs));
}
```
I also restructured the loop to dequeue first rather than testing `IsEmpty`
and then dequeuing, which removes the separate `timeout` flag.
Two regression tests added:
- `Take_RepeatedWakeUps_StillHonoursTheOverallDeadline` — a background task
pulses the monitor every 20 ms while the queue is **never** fed, so the waiter
is only ever woken spuriously. With a 200 ms budget it must still give up. I
verified this test against the pre-fix behaviour by temporarily pinning
`remainingMs = budgetMs`: the test hangs indefinitely instead of failing fast,
which is precisely the defect you described.
- `Take_MultipleWaiters_EachHonoursTheOverallDeadline` — four concurrent
waiters, one client returned; the three losers must each settle within their
own deadline.
I used spurious pulses rather than return/reclaim churn for the first test
because the latter was racy — the waiter occasionally won the client and the
test flaked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]