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]

Reply via email to