Skip to content

Commit 2f5d095

Browse files
arturobernalgarturobernalg
committed
Use connection socket timeout as default HTTP/2 per-stream idle/lifetime timeout and update example/tests accordingly.
1 parent 87791a0 commit 2f5d095

File tree

6 files changed

+93
-160
lines changed

6 files changed

+93
-160
lines changed

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/H2StreamTimeoutException.java‎

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
*/
5454
public class H2StreamTimeoutException extends SocketTimeoutException {
5555

56+
private static final long serialVersionUID = 1L;
57+
5658
private final int streamId;
5759
private final Timeout timeout;
5860
private final boolean idleTimeout;
@@ -64,38 +66,22 @@ public H2StreamTimeoutException(final String message, final int streamId, final
6466
this.idleTimeout = idleTimeout;
6567
}
6668

67-
/**
68-
* Returns the HTTP/2 stream identifier associated with this timeout.
69-
*
70-
* @return the stream id
71-
* @since 5.4
72-
*/
7369
public int getStreamId() {
7470
return streamId;
7571
}
7672

77-
/**
78-
* Returns the timeout value that was in effect when this exception
79-
* was raised.
80-
*
81-
* @return the timeout value
82-
* @since 5.4
83-
*/
8473
public Timeout getTimeout() {
8574
return timeout;
8675
}
8776

8877
/**
89-
* Indicates whether this exception was triggered by an idle timeout
90-
* (no activity on the stream) as opposed to an overall lifetime timeout.
78+
* Indicates whether this timeout was triggered by idle time (no activity)
79+
* rather than by stream lifetime.
9180
*
92-
* @return {@code true} if this is an idle timeout, {@code false}
93-
* if it is a lifetime timeout
94-
* @since 5.4
81+
* @return {@code true} if this is an idle timeout, {@code false} if it is a lifetime timeout.
9582
*/
9683
public boolean isIdleTimeout() {
9784
return idleTimeout;
9885
}
9986

10087
}
101-

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/config/H2Config.java‎

Lines changed: 3 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.apache.hc.core5.annotation.ThreadingBehavior;
3232
import org.apache.hc.core5.http2.frame.FrameConsts;
3333
import org.apache.hc.core5.util.Args;
34-
import org.apache.hc.core5.util.Timeout;
3534

3635
/**
3736
* HTTP/2 protocol configuration.
@@ -53,13 +52,9 @@ public class H2Config {
5352
private final boolean compressionEnabled;
5453
private final int maxContinuations;
5554

56-
private final Timeout streamIdleTimeout;
57-
private final Timeout streamLifetimeTimeout;
58-
5955
H2Config(final int headerTableSize, final boolean pushEnabled, final int maxConcurrentStreams,
6056
final int initialWindowSize, final int maxFrameSize, final int maxHeaderListSize,
61-
final boolean compressionEnabled, final int maxContinuations,
62-
final Timeout streamIdleTimeout, final Timeout streamLifetimeTimeout) {
57+
final boolean compressionEnabled, final int maxContinuations) {
6358
super();
6459
this.headerTableSize = headerTableSize;
6560
this.pushEnabled = pushEnabled;
@@ -69,8 +64,6 @@ public class H2Config {
6964
this.maxHeaderListSize = maxHeaderListSize;
7065
this.compressionEnabled = compressionEnabled;
7166
this.maxContinuations = maxContinuations;
72-
this.streamIdleTimeout = streamIdleTimeout;
73-
this.streamLifetimeTimeout = streamLifetimeTimeout;
7467
}
7568

7669
public int getHeaderTableSize() {
@@ -105,26 +98,6 @@ public int getMaxContinuations() {
10598
return maxContinuations;
10699
}
107100

108-
/**
109-
* Returns the per-stream idle timeout.
110-
*
111-
* @return the per-stream idle timeout, or {@code null} if not configured
112-
* @since 5.4
113-
*/
114-
public Timeout getStreamIdleTimeout() {
115-
return streamIdleTimeout;
116-
}
117-
118-
/**
119-
* Returns the maximum lifetime timeout for HTTP/2 streams.
120-
*
121-
* @return the per-stream lifetime timeout, or {@code null} if not configured
122-
* @since 5.4
123-
*/
124-
public Timeout getStreamLifetimeTimeout() {
125-
return streamLifetimeTimeout;
126-
}
127-
128101
@Override
129102
public String toString() {
130103
final StringBuilder builder = new StringBuilder();
@@ -136,8 +109,6 @@ public String toString() {
136109
.append(", maxHeaderListSize=").append(this.maxHeaderListSize)
137110
.append(", compressionEnabled=").append(this.compressionEnabled)
138111
.append(", maxContinuations=").append(this.maxContinuations)
139-
.append(", streamIdleTimeout=").append(this.streamIdleTimeout)
140-
.append(", streamLifetimeTimeout=").append(this.streamLifetimeTimeout)
141112
.append("]");
142113
return builder.toString();
143114
}
@@ -171,9 +142,7 @@ public static H2Config.Builder copy(final H2Config config) {
171142
.setInitialWindowSize(config.getInitialWindowSize())
172143
.setMaxFrameSize(config.getMaxFrameSize())
173144
.setMaxHeaderListSize(config.getMaxHeaderListSize())
174-
.setCompressionEnabled(config.isCompressionEnabled())
175-
.setStreamIdleTimeout(config.getStreamIdleTimeout())
176-
.setStreamLifetimeTimeout(config.getStreamLifetimeTimeout());
145+
.setCompressionEnabled(config.isCompressionEnabled());
177146
}
178147

179148
public static class Builder {
@@ -186,8 +155,6 @@ public static class Builder {
186155
private int maxHeaderListSize;
187156
private boolean compressionEnabled;
188157
private int maxContinuations;
189-
private Timeout streamIdleTimeout;
190-
private Timeout streamLifetimeTimeout;
191158

192159
Builder() {
193160
this.headerTableSize = INIT_HEADER_TABLE_SIZE * 2;
@@ -252,48 +219,6 @@ public Builder setMaxContinuations(final int maxContinuations) {
252219
return this;
253220
}
254221

255-
/**
256-
* Sets the per-stream idle timeout.
257-
* <p>
258-
* When enabled and greater than zero, an active HTTP/2 stream that has no
259-
* activity (no headers or data frames) for longer than the configured
260-
* timeout will be reset by the HTTP/2 multiplexer.
261-
* </p>
262-
* <p>
263-
* Use {@code null} or a disabled {@link Timeout} value to turn this
264-
* feature off.
265-
* </p>
266-
*
267-
* @param timeout the per-stream idle timeout; may be {@code null}
268-
* @return this builder
269-
* @since 5.4
270-
*/
271-
public Builder setStreamIdleTimeout(final Timeout timeout) {
272-
this.streamIdleTimeout = timeout;
273-
return this;
274-
}
275-
276-
/**
277-
* Sets the maximum lifetime of an HTTP/2 stream.
278-
* <p>
279-
* When enabled and greater than zero, an active HTTP/2 stream whose age
280-
* (time since creation) exceeds the configured timeout will be reset by
281-
* the HTTP/2 multiplexer, regardless of recent activity.
282-
* </p>
283-
* <p>
284-
* Use {@code null} or a disabled {@link Timeout} value to turn this
285-
* feature off.
286-
* </p>
287-
*
288-
* @param timeout the per-stream lifetime timeout; may be {@code null}
289-
* @return this builder
290-
* @since 5.4
291-
*/
292-
public Builder setStreamLifetimeTimeout(final Timeout timeout) {
293-
this.streamLifetimeTimeout = timeout;
294-
return this;
295-
}
296-
297222
public H2Config build() {
298223
return new H2Config(
299224
headerTableSize,
@@ -303,9 +228,7 @@ public H2Config build() {
303228
maxFrameSize,
304229
maxHeaderListSize,
305230
compressionEnabled,
306-
maxContinuations,
307-
streamIdleTimeout,
308-
streamLifetimeTimeout);
231+
maxContinuations);
309232
}
310233

311234
}

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,6 @@ enum SettingsHandshake { READY, TRANSMITTED, ACKED }
145145
private final Map<Integer, PriorityValue> priorities = new ConcurrentHashMap<>();
146146
private volatile boolean peerNoRfc7540Priorities;
147147

148-
private final Timeout streamIdleTimeout;
149-
private final Timeout streamLifetimeTimeout;
150-
151148
AbstractH2StreamMultiplexer(
152149
final ProtocolIOSession ioSession,
153150
final FrameFactory frameFactory,
@@ -182,8 +179,6 @@ enum SettingsHandshake { READY, TRANSMITTED, ACKED }
182179

183180
this.lowMark = H2Config.INIT.getInitialWindowSize() / 2;
184181
this.streamListener = streamListener;
185-
this.streamIdleTimeout = this.localConfig.getStreamIdleTimeout();
186-
this.streamLifetimeTimeout = this.localConfig.getStreamLifetimeTimeout();
187182
}
188183

189184
@Override
@@ -661,6 +656,7 @@ private void executeRequest(final RequestExecutionCommand requestExecutionComman
661656
requestExecutionCommand.getExchangeHandler(),
662657
requestExecutionCommand.getPushHandlerFactory(),
663658
requestExecutionCommand.getContext()));
659+
initializeStreamTimeouts(stream);
664660

665661
if (streamListener != null) {
666662
final int initInputWindow = stream.getInputWindow().get();
@@ -783,10 +779,12 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
783779
final H2StreamChannel channel = createChannel(streamId);
784780
if (connState.compareTo(ConnectionHandshake.ACTIVE) <= 0) {
785781
stream = streams.createActive(channel, incomingRequest(channel));
782+
initializeStreamTimeouts(stream);
786783
streams.resetIfExceedsMaxConcurrentLimit(stream, localConfig.getMaxConcurrentStreams());
787784
} else {
788785
channel.localReset(H2Error.REFUSED_STREAM);
789786
stream = streams.createActive(channel, NoopH2StreamHandler.INSTANCE);
787+
initializeStreamTimeouts(stream);
790788
}
791789
} else if (stream.isLocalClosed() && stream.isRemoteClosed()) {
792790
throw new H2ConnectionException(H2Error.STREAM_CLOSED, "Stream closed");
@@ -977,6 +975,7 @@ private void consumeFrame(final RawFrame frame) throws HttpException, IOExceptio
977975
channel.localReset(H2Error.REFUSED_STREAM);
978976
promisedStream = streams.createActive(channel, NoopH2StreamHandler.INSTANCE);
979977
}
978+
initializeStreamTimeouts(promisedStream);
980979
try {
981980
consumePushPromiseFrame(frame, payload, promisedStream);
982981
} catch (final H2StreamResetException ex) {
@@ -1382,8 +1381,18 @@ H2StreamChannel createChannel(final int streamId) {
13821381
return new H2StreamChannelImpl(streamId, initInputWinSize, initOutputWinSize);
13831382
}
13841383

1385-
H2Stream createStream(final H2StreamChannel channel, final H2StreamHandler streamHandler) throws H2ConnectionException {
1386-
return streams.createActive(channel, streamHandler);
1384+
private void initializeStreamTimeouts(final H2Stream stream) {
1385+
final Timeout socketTimeout = ioSession.getSocketTimeout();
1386+
if (socketTimeout != null && socketTimeout.isEnabled()) {
1387+
stream.setIdleTimeout(socketTimeout);
1388+
stream.setLifetimeTimeout(socketTimeout);
1389+
}
1390+
}
1391+
1392+
H2Stream createStream(final H2StreamChannel channel, final H2StreamHandler streamHandler) {
1393+
final H2Stream stream = streams.createActive(channel, streamHandler);
1394+
initializeStreamTimeouts(stream);
1395+
return stream;
13871396
}
13881397

13891398
private void recordPriorityFromHeaders(final int streamId, final List<? extends Header> headers) {
@@ -1486,6 +1495,7 @@ public void push(final List<Header> headers, final AsyncPushProducer pushProduce
14861495
final int promisedStreamId = streams.generateStreamId();
14871496
final H2StreamChannel channel = createChannel(promisedStreamId);
14881497
final H2Stream stream = streams.createReserved(channel, outgoingPushPromise(channel, pushProducer));
1498+
initializeStreamTimeouts(stream);
14891499

14901500
commitPushPromise(id, promisedStreamId, headers);
14911501
stream.markRemoteClosed();
@@ -1603,45 +1613,45 @@ public String toString() {
16031613
}
16041614

16051615
private void checkStreamTimeouts(final long nowNanos) throws IOException {
1606-
if ((streamIdleTimeout == null || !streamIdleTimeout.isEnabled())
1607-
&& (streamLifetimeTimeout == null || !streamLifetimeTimeout.isEnabled())) {
1608-
return;
1609-
}
1610-
16111616
for (final Iterator<H2Stream> it = streams.iterator(); it.hasNext(); ) {
16121617
final H2Stream stream = it.next();
16131618
if (!stream.isActive()) {
16141619
continue;
16151620
}
16161621

1622+
final Timeout idleTimeout = stream.getIdleTimeout();
1623+
final Timeout lifetimeTimeout = stream.getLifetimeTimeout();
1624+
if ((idleTimeout == null || !idleTimeout.isEnabled())
1625+
&& (lifetimeTimeout == null || !lifetimeTimeout.isEnabled())) {
1626+
continue;
1627+
}
1628+
16171629
final long created = stream.getCreatedNanos();
16181630
final long last = stream.getLastActivityNanos();
16191631

1620-
// Idle timeout
1621-
if (streamIdleTimeout != null && streamIdleTimeout.isEnabled()) {
1622-
final long idleNanos = streamIdleTimeout.toNanoseconds();
1632+
if (idleTimeout != null && idleTimeout.isEnabled()) {
1633+
final long idleNanos = idleTimeout.toNanoseconds();
16231634
if (idleNanos > 0 && nowNanos - last > idleNanos) {
16241635
final int streamId = stream.getId();
16251636
final H2StreamTimeoutException ex = new H2StreamTimeoutException(
1626-
"HTTP/2 stream idle timeout (" + streamIdleTimeout + ")",
1637+
"HTTP/2 stream idle timeout (" + idleTimeout + ")",
16271638
streamId,
1628-
streamIdleTimeout,
1639+
idleTimeout,
16291640
true);
16301641
stream.localReset(ex, H2Error.CANCEL);
16311642
// Once reset due to idle timeout, we do not care about lifetime anymore
16321643
continue;
16331644
}
16341645
}
16351646

1636-
// Lifetime timeout
1637-
if (streamLifetimeTimeout != null && streamLifetimeTimeout.isEnabled()) {
1638-
final long lifeNanos = streamLifetimeTimeout.toNanoseconds();
1647+
if (lifetimeTimeout != null && lifetimeTimeout.isEnabled()) {
1648+
final long lifeNanos = lifetimeTimeout.toNanoseconds();
16391649
if (lifeNanos > 0 && nowNanos - created > lifeNanos) {
16401650
final int streamId = stream.getId();
16411651
final H2StreamTimeoutException ex = new H2StreamTimeoutException(
1642-
"HTTP/2 stream lifetime timeout (" + streamLifetimeTimeout + ")",
1652+
"HTTP/2 stream lifetime timeout (" + lifetimeTimeout + ")",
16431653
streamId,
1644-
streamLifetimeTimeout,
1654+
lifetimeTimeout,
16451655
false);
16461656
stream.localReset(ex, H2Error.CANCEL);
16471657
}

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/H2Stream.java‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.apache.hc.core5.http.nio.HandlerFactory;
4444
import org.apache.hc.core5.http2.H2Error;
4545
import org.apache.hc.core5.http2.H2StreamResetException;
46+
import org.apache.hc.core5.util.Timeout;
4647

4748
class H2Stream {
4849

@@ -63,6 +64,9 @@ enum State { RESERVED, OPEN, CLOSED }
6364
private volatile long createdNanos;
6465
private volatile long lastActivityNanos;
6566

67+
private volatile Timeout idleTimeout;
68+
private volatile Timeout lifetimeTimeout;
69+
6670
H2Stream(final H2StreamChannel channel, final H2StreamHandler handler, final Consumer<State> stateChangeCallback) {
6771
this.channel = channel;
6872
this.handler = handler;
@@ -101,7 +105,6 @@ void activate() {
101105
triggerOpen();
102106
}
103107

104-
105108
AtomicInteger getOutputWindow() {
106109
return channel.getOutputWindow();
107110
}
@@ -141,7 +144,6 @@ boolean isLocalClosed() {
141144

142145
void consumePromise(final List<Header> headers) throws HttpException, IOException {
143146
try {
144-
// NEW
145147
touch();
146148

147149
if (channel.isLocalReset()) {
@@ -219,7 +221,6 @@ void produceInputCapacityUpdate() throws IOException {
219221
handler.updateInputCapacity();
220222
}
221223

222-
223224
void fail(final Exception cause) {
224225
remoteClosed = true;
225226
channel.markLocalClosed();
@@ -324,4 +325,20 @@ long getCreatedNanos() {
324325
long getLastActivityNanos() {
325326
return lastActivityNanos;
326327
}
328+
329+
Timeout getIdleTimeout() {
330+
return idleTimeout;
331+
}
332+
333+
void setIdleTimeout(final Timeout idleTimeout) {
334+
this.idleTimeout = idleTimeout;
335+
}
336+
337+
Timeout getLifetimeTimeout() {
338+
return lifetimeTimeout;
339+
}
340+
341+
void setLifetimeTimeout(final Timeout lifetimeTimeout) {
342+
this.lifetimeTimeout = lifetimeTimeout;
343+
}
327344
}

0 commit comments

Comments
 (0)