From a091d9df4ea4519ce3a5cc3dc56b7b299c6c5bba Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Thu, 23 Apr 2020 11:25:39 +0200
Subject: [PATCH] HTTP2: Guard against multiple ctx.close(...) calls with the
 same ChannelPromise (#10201)

Motivation:

Http2ConnectionHandler may call ctx.close(...) with the same promise instance multiple times if the timeout for gracefulShutdown elapse and the listener itself is notified. This can cause problems as other handlers in the pipeline may queue these promises and try to notify these later via setSuccess() or setFailure(...) which will then throw an IllegalStateException if the promise was notified already

Modification:

- Add boolean flag to ensure doClose() will only try to call ctx.close(...) one time

Result:

Don't call ctx.close(...) with the same promise multiple times when gradefulShutdown timeout elapses.
---
 .../handler/codec/http2/Http2ConnectionHandler.java      | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
index f034345887..df7b6ef2d1 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
@@ -915,6 +915,7 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
         private final ChannelHandlerContext ctx;
         private final ChannelPromise promise;
         private final ScheduledFuture<?> timeoutTask;
+        private boolean closed;
 
         ClosingChannelFutureListener(ChannelHandlerContext ctx, ChannelPromise promise) {
             this.ctx = ctx;
@@ -938,6 +939,14 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
         }
 
         private void doClose() {
+            // We need to guard against multiple calls as the timeout may trigger close() first and then it will be
+            // triggered again because of operationComplete(...) is called.
+            if (closed) {
+                // This only happens if we also scheduled a timeout task.
+                assert timeoutTask != null;
+                return;
+            }
+            closed = true;
             if (promise == null) {
                 ctx.close();
             } else {