The issue was observed on a local run in PacketSpaceManagerTest:

org.opentest4j.AssertionFailedError: nextDeadline should not be after 250ms 
from now! ==> expected: <false> but was: <true>
        at 
org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at 
org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
        at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
        at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:239)
        at 
PacketSpaceManagerTest$SynchronousTestDriver.run(PacketSpaceManagerTest.java:856)

Investigation suggests that this is caused by a race condition where 
`packetSent` was invoked while the transmitter task was running: `packetSent` 
will only reschedule the transmitter if there is no deadline, that is, if it 
observes that `deadline == Deadline.MAX`; But if `deadline` is already in the 
past, which is typically the case when the transmitter task is running, then 
there's no guarantee that the transmitter task will actually end up 
rescheduling itself with a new deadline. It may not if it thinks there is 
nothing pending. The transmitter will then remain unscheduled until something 
else happens that causes it to be rescheduled.

In a typical HTTP/3 request/response exchange chances are high that "something 
else" will happen, which explains why even if `packetSent` fails to reschedule, 
there might not be any observable consequence (outside of 
PacketSpaceManagerTest). However, there is no guarantee.

The fix is to always call PacketSpaceManager::reschedule, which will compute a 
new prospective deadline which the timer queue can then reliably use to decide 
whether the selector should be woken up to take into account the new deadline.

A new testPacketSent() test that deterministically fails without the fix is 
added to  PacketSpaceManagerTest.

---------
- [x] I confirm that I make this contribution in accordance with the [OpenJDK 
Interim AI Policy](https://openjdk.org/legal/ai).

-------------

Commit messages:
 - Add more test comments
 - 8386985: PacketSpaceManagerTest failed with AssertionError; A race condition 
may cause packetSent to mistakenly skip rescheduling of the transmitter task

Changes: https://git.openjdk.org/jdk/pull/31662/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=31662&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8386985
  Stats: 104 lines in 2 files changed: 75 ins; 22 del; 7 mod
  Patch: https://git.openjdk.org/jdk/pull/31662.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/31662/head:pull/31662

PR: https://git.openjdk.org/jdk/pull/31662

Reply via email to