infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-07-02 15:20:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

8386985: PacketSpaceManagerTest failed with AssertionError; A race condition may cause packetSent to mistakenly skip rescheduling of the transmitter task

Browse Source 
Reviewed-by: djelinski
This commit is contained in:
Daniel Fuchs 2026-06-29 14:18:48 +00:00
parent cc83fbd132
commit f740f7c66b
2 changed files with 82 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/java.net.http/share/classes/jdk/internal/net/http/quic/PacketSpaceManager.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -645,22 +645,15 @@ public sealed class PacketSpaceManager implements PacketSpace
return false;
}
boolean hasNoDeadline() {
return Deadline.MAX.equals(nextDeadline);
}
// reschedule this task
void reschedule() {
Deadline deadline = computeNextDeadline();
Deadline nextDeadline = this.nextDeadline;
if (Deadline.MAX.equals(deadline)) {
debug.log("no deadline, don't reschedule");
} else if (deadline.equals(nextDeadline)) {
debug.log("deadline unchanged, don't reschedule");
} else {
packetEmitter.reschedule(this, deadline);
debug.log("retransmission task: rescheduled");
if (debug.on()) debug.log("no deadline, don't reschedule");
return;
}
if (debug.on()) debug.log("retransmission task: rescheduled");
packetEmitter.reschedule(this, deadline);
}
@Override
@ -1304,7 +1297,7 @@ public sealed class PacketSpaceManager implements PacketSpace
} finally {
transferLock.unlock();
}
if (found && packetTransmissionTask.hasNoDeadline()) {
if (found) {
packetTransmissionTask.reschedule();
}
if (!found) {
@ -1340,9 +1333,7 @@ public sealed class PacketSpaceManager implements PacketSpace
return;
}
addAcknowledgement(pending);
if (packetTransmissionTask.hasNoDeadline()) {
packetTransmissionTask.reschedule();
}
packetTransmissionTask.reschedule();
} finally {
transferLock.unlock();
}

88
test/jdk/java/net/httpclient/quic/PacketSpaceManagerTest.java
View File

@ -80,16 +80,19 @@ import javax.net.ssl.SSLSession;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
/*
* @test
* @bug 8349910 8386985
* @summary tests the logic to build an AckFrame
* @library /test/lib
* @library ../debug
@ -102,6 +105,7 @@ import org.junit.jupiter.params.provider.MethodSource;
* @run junit/othervm -Dseed=-4159871071396382784 ${test.main.class}
* @run junit/othervm -Dseed=2252276218459363615 ${test.main.class}
* @run junit/othervm -Dseed=-5130588140709404919 ${test.main.class}
* @run junit/othervm -Dseed=4257295716830862528 ${test.main.class}
*/
// -Djdk.internal.httpclient.debug=true
public class PacketSpaceManagerTest {
@ -766,6 +770,26 @@ public class PacketSpaceManagerTest {
}
}
// Sends a trivial INITIAL packet, with a CRYPTO frame containing
// a payload of length 1 for the provided offset. If ackFrameToSend
// is not null it will be included in the packet.
public List<QuicFrame> sendPacket(long offset, AckFrame ackFrameToSend, Packet packet, long largestReceivedAckedPN) {
// add a crypto frame and build the packet
CryptoFrame crypto = new CryptoFrame(offset, 1,
ByteBuffer.wrap(new byte[] {nextByte(offset)}));
List<QuicFrame> frames = ackFrameToSend == null ?
List.of(crypto) : List.of(crypto, ackFrameToSend);
QuicPacket newPacket = codingContext.encoder
.newInitialPacket(localId, peerId,
null,
packet.packetNumber,
largestReceivedAckedPN,
frames, codingContext);
// pretend that we sent a packet
manager.packetSent(newPacket, -1, packet.packetNumber);
return frames;
}
/**
* Drives the test by pretending to emit each packet in order,
* then pretending to receive ack frames (as soon as possible
@ -830,19 +854,8 @@ public class PacketSpaceManagerTest {
debug.log("largestAckSent is: " + largestAckAcked);
}
// add a crypto frame and build the packet
CryptoFrame crypto = new CryptoFrame(offset, 1,
ByteBuffer.wrap(new byte[] {nextByte(offset)}));
List<QuicFrame> frames = ackFrameToSend == null ?
List.of(crypto) : List.of(crypto, ackFrameToSend);
QuicPacket newPacket = codingContext.encoder
.newInitialPacket(localId, peerId,
null,
packet.packetNumber,
largestReceivedAckedPN,
frames, codingContext);
// pretend that we sent a packet
manager.packetSent(newPacket, -1, packet.packetNumber);
// send a packet
List<QuicFrame> frames = sendPacket(offset, ackFrameToSend, packet, largestReceivedAckedPN);
// compute next deadline
var nextDeadline = timerQueue.nextDeadline();
@ -1125,4 +1138,53 @@ public class PacketSpaceManagerTest {
driver.check();
}
@Test
public void testPacketSent() throws Exception {
// this test case is specifically for JDK-8386985
System.out.printf("%n ------- testPacketSent ------- %n");
// create a minimal SynchronousTestDriver
TestCase testCase = new TestCase(List.of(new Acknowledged(1, 3), new Acknowledged(4,4)),
List.of(new Packet(3, 0), new Packet(4, 0)));
SynchronousTestDriver driver = new SynchronousTestDriver(testCase);
// send a first ack-eliciting packet, and move the timeline past PTO
driver.sendPacket(0, null, new Packet(1, 0), -1);
Deadline pto = driver.manager.nextScheduledDeadline(); // should be PTO
driver.timeSource.advance(driver.timeSource.instant().until(pto, ChronoUnit.MILLIS) + 250, ChronoUnit.MILLIS);
// start processing events, but delay the task that will run the transmitter
ArrayList<Runnable> tasks = new ArrayList<>();
Executor executor = new Executor() {
@Override
public void execute(Runnable command) {
tasks.add(command);
}
};
driver.timerQueue.processEventsAndReturnNextDeadline(driver.timeSource.instant(), executor);
// acknowledge the first packet so that it's no longer pending retransmission
driver.manager.processAckFrame(new AckFrameBuilder().addAck(1).build());
// send a second packet, and examine the timerQueue next deadline
// if sending the second packet didn't cause the task to be rescheduled, we
// will observe Deadline.MAX, or a deadline before now: that's the bug.
driver.sendPacket(1, null, new Packet(2, 0), -1);
Deadline next = driver.timerQueue.nextDeadline();
assertNotEquals(next, Deadline.MAX);
assertTrue(next.isAfter(driver.timeSource.instant()));
// now finish running the task and ack the second packet,
// so that we leave the packet space manager in a clean state
// for running the driver with the next two packets.
for (Runnable task : tasks) {
task.run();
}
driver.manager.processAckFrame(new AckFrameBuilder()
.addAck(1).addAck(2).build());
driver.run();
driver.check();
}
}