Skip to content

Commit d163c06

Browse files
authored
servlet: add extra 5s to AsyncContext timeout (grpc#12321)
Currently there is a race between 2 tasks scheduled to handle request timeout * servlet AsyncContext timeout * gRPC Context which can cause instability. This change makes the gRPC Context timeout happen first.
1 parent 6f21bc2 commit d163c06

File tree

2 files changed

+11
-5
lines changed

2 files changed

+11
-5
lines changed

servlet/src/main/java/io/grpc/servlet/ServletAdapter.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static java.util.logging.Level.FINE;
2323
import static java.util.logging.Level.FINEST;
2424

25+
import com.google.common.annotations.VisibleForTesting;
2526
import com.google.common.io.BaseEncoding;
2627
import io.grpc.Attributes;
2728
import io.grpc.ExperimentalApi;
@@ -134,10 +135,9 @@ public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOEx
134135
}
135136

136137
Long timeoutNanos = headers.get(TIMEOUT_KEY);
137-
if (timeoutNanos == null) {
138-
timeoutNanos = 0L;
139-
}
140-
asyncCtx.setTimeout(TimeUnit.NANOSECONDS.toMillis(timeoutNanos));
138+
asyncCtx.setTimeout(timeoutNanos != null
139+
? TimeUnit.NANOSECONDS.toMillis(timeoutNanos) + ASYNC_TIMEOUT_SAFETY_MARGIN
140+
: 0);
141141
StatsTraceContext statsTraceCtx =
142142
StatsTraceContext.newServerContext(streamTracerFactories, method, headers);
143143

@@ -164,6 +164,12 @@ public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOEx
164164
asyncCtx.addListener(new GrpcAsyncListener(stream, logId));
165165
}
166166

167+
/**
168+
* Deadlines are managed via Context, servlet async timeout is not supposed to happen.
169+
*/
170+
@VisibleForTesting
171+
static final long ASYNC_TIMEOUT_SAFETY_MARGIN = 5_000;
172+
167173
// This method must use Enumeration and its members, since that is the only way to read headers
168174
// from the servlet api.
169175
@SuppressWarnings("JdkObsolete")

servlet/src/test/java/io/grpc/servlet/ServletServerBuilderTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public void scheduledExecutorService() throws Exception {
8080
ServletAdapter servletAdapter = serverBuilder.buildServletAdapter();
8181
servletAdapter.doPost(request, response);
8282

83-
verify(asyncContext).setTimeout(1);
83+
verify(asyncContext).setTimeout(1 + ServletAdapter.ASYNC_TIMEOUT_SAFETY_MARGIN);
8484

8585
// The following just verifies that scheduler is populated to the transport.
8686
// It doesn't matter what tasks (such as handshake timeout and request deadline) are actually

0 commit comments

Comments
 (0)