From 5bc644ba41c3c2f0eb1a12622b1854c3d620b29f Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Fri, 7 Feb 2020 09:14:16 +0100
Subject: [PATCH] Ensure ChannelOptions are applied in the same order as
 configured in Http2StreamChannelBootstrap (#9998) (#10001)

Motivation:

https://github.com/netty/netty/pull/9848 changed how we handled ChannelOptions internally to use a ConcurrentHashMap. This unfortunally had the side-effect that the ordering may be affected and not stable anymore. Here the problem is that sometimes we do validation based on two different ChannelOptions (for example we validate high and low watermarks against each other). Thus even if the user specified the options in the same order we may fail to configure them.

Modifications:

- Use again a LinkedHashMap to preserve order

Result:

Apply ChannelOptions in correct and expected order
---
 .../http2/Http2StreamChannelBootstrap.java    | 23 ++++++++++++++-----
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java
index 60d7509383..f3007ff7a2 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java
@@ -33,6 +33,7 @@ import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
 import java.nio.channels.ClosedChannelException;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -44,7 +45,9 @@ public final class Http2StreamChannelBootstrap {
     @SuppressWarnings("unchecked")
     private static final Map.Entry<AttributeKey<?>, Object>[] EMPTY_ATTRIBUTE_ARRAY = new Map.Entry[0];
 
-    private final Map<ChannelOption<?>, Object> options = new ConcurrentHashMap<ChannelOption<?>, Object>();
+    // The order in which ChannelOptions are applied is important they may depend on each other for validation
+    // purposes.
+    private final Map<ChannelOption<?>, Object> options = new LinkedHashMap<ChannelOption<?>, Object>();
     private final Map<AttributeKey<?>, Object> attrs = new ConcurrentHashMap<AttributeKey<?>, Object>();
     private final Channel channel;
     private volatile ChannelHandler handler;
@@ -63,10 +66,13 @@ public final class Http2StreamChannelBootstrap {
     @SuppressWarnings("unchecked")
     public <T> Http2StreamChannelBootstrap option(ChannelOption<T> option, T value) {
         ObjectUtil.checkNotNull(option, "option");
-        if (value == null) {
-            options.remove(option);
-        } else {
-            options.put(option, value);
+
+        synchronized (options) {
+            if (value == null) {
+                options.remove(option);
+            } else {
+                options.put(option, value);
+            }
         }
         return this;
     }
@@ -202,7 +208,12 @@ public final class Http2StreamChannelBootstrap {
         if (handler != null) {
             p.addLast(handler);
         }
-        setChannelOptions(channel, options.entrySet().toArray(EMPTY_OPTION_ARRAY));
+        final Map.Entry<ChannelOption<?>, Object> [] optionArray;
+        synchronized (options) {
+            optionArray = options.entrySet().toArray(EMPTY_OPTION_ARRAY);
+        }
+
+        setChannelOptions(channel, optionArray);
         setAttributes(channel, attrs.entrySet().toArray(EMPTY_ATTRIBUTE_ARRAY));
     }