Skip to content

Commit 7f748eb

Browse files
committed
Expand HTTP/2 timeout handling to connection window exhaustion on write.
1 parent 151b447 commit 7f748eb

File tree

3 files changed

+50
-13
lines changed

3 files changed

+50
-13
lines changed

java/org/apache/coyote/http2/Http2UpgradeHandler.java

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -801,7 +801,26 @@ int reserveWindowSize(Stream stream, int reservation, boolean block) throws IOEx
801801
if (allocation == 0) {
802802
if (block) {
803803
try {
804-
stream.wait();
804+
// Connection level window is empty. Although this
805+
// request is for a stream, use the connection
806+
// timeout
807+
long writeTimeout = protocol.getWriteTimeout();
808+
if (writeTimeout < 0) {
809+
stream.wait();
810+
} else {
811+
stream.wait(writeTimeout);
812+
}
813+
// Has this stream been granted an allocation
814+
int[] value = backLogStreams.get(stream);
815+
if (value[1] == 0) {
816+
// No allocation
817+
// Close the connection. Do this first since
818+
// closing the stream will raise an exception
819+
close();
820+
// Close the stream (in app code so need to
821+
// signal to app stream is closing)
822+
stream.doWriteTimeout();
823+
}
805824
} catch (InterruptedException e) {
806825
throw new IOException(sm.getString(
807826
"upgradeHandler.windowSizeReservationInterrupted", connectionId,
@@ -1024,11 +1043,20 @@ private Stream createLocalStream(Request request) {
10241043

10251044

10261045
private void close() {
1027-
connectionState.set(ConnectionState.CLOSED);
1046+
ConnectionState previous = connectionState.getAndSet(ConnectionState.CLOSED);
1047+
if (previous == ConnectionState.CLOSED) {
1048+
// Already closed
1049+
return;
1050+
}
1051+
10281052
for (Stream stream : streams.values()) {
10291053
// The connection is closing. Close the associated streams as no
10301054
// longer required.
10311055
stream.receiveReset(Http2Error.CANCEL.getCode());
1056+
// Release any streams waiting for an allocation
1057+
synchronized (stream) {
1058+
stream.notifyAll();
1059+
}
10321060
}
10331061
try {
10341062
socketWrapper.close();

java/org/apache/coyote/http2/Stream.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -283,17 +283,7 @@ final synchronized int reserveWindowSize(int reservation, boolean block)
283283
}
284284
windowSize = getWindowSize();
285285
if (windowSize == 0) {
286-
String msg = sm.getString("stream.writeTimeout");
287-
StreamException se = new StreamException(
288-
msg, Http2Error.ENHANCE_YOUR_CALM, getIdAsInt());
289-
// Prevent the application making further writes
290-
streamOutputBuffer.closed = true;
291-
// Prevent Tomcat's error handling trying to write
292-
coyoteResponse.setError();
293-
coyoteResponse.setErrorReported();
294-
// Trigger a reset once control returns to Tomcat
295-
streamOutputBuffer.reset = se;
296-
throw new CloseNowException(msg, se);
286+
doWriteTimeout();
297287
}
298288
} catch (InterruptedException e) {
299289
// Possible shutdown / rst or similar. Use an IOException to
@@ -316,6 +306,21 @@ final synchronized int reserveWindowSize(int reservation, boolean block)
316306
}
317307

318308

309+
void doWriteTimeout() throws CloseNowException {
310+
String msg = sm.getString("stream.writeTimeout");
311+
StreamException se = new StreamException(
312+
msg, Http2Error.ENHANCE_YOUR_CALM, getIdAsInt());
313+
// Prevent the application making further writes
314+
streamOutputBuffer.closed = true;
315+
// Prevent Tomcat's error handling trying to write
316+
coyoteResponse.setError();
317+
coyoteResponse.setErrorReported();
318+
// Trigger a reset once control returns to Tomcat
319+
streamOutputBuffer.reset = se;
320+
throw new CloseNowException(msg, se);
321+
}
322+
323+
319324
@Override
320325
public final void emitHeader(String name, String value) throws HpackException {
321326
if (log.isDebugEnabled()) {

webapps/docs/changelog.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,10 @@
171171
for possible use with an asynchronous workflow like CompletableFuture.
172172
(remm)
173173
</update>
174+
<fix>
175+
Expand HTTP/2 timeout handling to include connection window exhaustion
176+
on write. (markt)
177+
</fix>
174178
</changelog>
175179
</subsection>
176180
<subsection name="Jasper">

0 commit comments

Comments
 (0)