Repository: netty/netty
Issue: Clean up HTTP/2 MAX_CONCURRENT_STREAMS logic
Labels: improvement

There are several methods on `Http2Connection.Endpoint` that involve this parameter:

`acceptingNewStreams`
`createStream`
`reservePushStream`
`maxStreams` (getter and setter)

It is not always clear which direction the value applies to.  It's probably worth a pass to:

1) Make sure we're actually using it correctly in all places.
2) See if we can make the interface more clear as to the directionality of the parameter


Issue comments:
@nmittler - Do you have any suggestions to improve clarity?  By `direction` do you mean local/remote? The only thing that may be tough is the Endpoint interface is agnostic to direction due to it serving as both remote/local.

@Scottmitch I haven't put much thought into it yet :).  For push, we do clarify direction with the method name `endpoint.allowPushTo()`.  Perhaps something like `endpoint.maxConcurrentStreamsFromPeer()` or `endpoint.maxRemotelyInitiatedConcurrentStreams`?

This would more clearly map to the `SETTINGS_MAX_CONCURRENT_STREAM` parameter from the spec.  It would, however, invert our current logic which maintains the value as the `maxStreams` that can be created by the endpoint.

WDYT?

@nmittler - Seems a bit overloaded/confusing if the `remote` (or `local`) endpoint has a `remote` method.  In this context it already takes a second to get oriented when the "local" settings uses the "remote" endpoint's `maxStreams` [here](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java#L146)...and if we add more directionality...does that conceptually invert the relationship :)  A bit of devils advocate but I just want to be sure we can clarify the situation by adding more directionality context.

@Scottmitch yes, the idea would be to invert the relationship so that the "local" settings use the local endpoint for everything. I think this would help to make things more clear for everyone wading through that code.

Naming is always tricky and I agree with all of your comments.  Suggestions are definitely welcome! :)

@nmittler - Ok thanks for clarifying.  I think I understand the scope of your suggestion...thought we were just changing names and keeping existing structure.  Sounds like we are in agreement...I will think on it.

@Scottmitch @buchgr here are my proposed changes to the `Endpoint` interface:

`createStream` (no change)

`acceptingNewStreams` becomes `canCreateStream`, and is documented to say that it compares against `maxConcurrentStreams`.

`numActiveStreams` becomes `concurrentStreams` and is documented to say the number of concurrent streams created by this endpoint.

`maxStreams` becomes `maxConcurrentStreams` and is documented to say that it's the number of concurrently active streams allowed to be created by this endpoint (i.e. the SETTINGS_MAX_CONCURRENT_STREAMS value provided by the opposite endpoint). This essentially keeps the logic as-is, but I think the documentation along with the other name changes help to clarify things a bit.  I considered having it represent the max concurrent streams allowed by the opposite endpoint, but all of the other methods refer to creating streams from this endpoint so I think it makes sense the way it is.

Thoughts?

@Scottmitch any thoughts on this?  I think this change may be less controversial than the #3491 :)

@nmittler - I'm sorry for the delay :(  +1 to your most recent suggestions.


Pull request review comments:
I think we have a naming disconnect between the `activate`/`deactivate` on the stream and `concurrent` on the endpoint.  For example the stream still has `numActiveStreams()` and `activeStreams()` which is logically the same thing as the endpoint's `numConcurrentStreams()` but with a different name. Can make this consistent?

I just double checked with the HTTP/2 spec and it sounds like our notion of "active" is exactly what should count towards `SETTINGS_MAX_CONCURRENT_STREAMS`.  Since the spec doesn't really provide a term for streams in this state (i.e. open or half-open), I think "active" is reasonable.  So I've changed everything over to use the "active" terminology.  I also cleaned up a few other things related to the Http2Connection API as well.

why we need the Math.min(...) at all ? 

@normanmaurer I'm assuming your suggestion is to directly cast the long to an int here? Wouldn't that allow us to get a negative value?

Shouldn't we have a `try { .. }` around this?  If 1 listener has an issue that shouldn't prevent us from notifying other listeners.

This may be a general comment when ever we notify listeners...another issue perhaps?

I thought the explicit "old" style loop was to avoid garbage being created from iterators and other objects?

@Scottmitch +1... We should only use for each if we either use a LinkedList or an array

`open` -> `{@code OPEN}`
`half-closed` -> `{@code HALF CLOSED}`

`open` -> `{@code OPEN}`
`half-closed` -> `{@code HALF CLOSED}`

+1. I like this change...no need to duplicate the interface at this level.

Was there a reason why the deactivation and state change was decoupled?  Was this so priority events would still be distributed to streams in this state?

But in this case since the `try { ...}` was added to the outer scope we may just want to remove it from this PR and address all instances of this issue in another PR...or just address them all in this PR?

Agreed, I think we should address this as a separate issue ... created #3501. 

WRT the finally block, I think I'd like to keep it in this PR since not doing this could lead to a memory leak if a listener throws.

Ah right ... I was trying to make intellij green :) Reverted this change.

Done ... scrubbed these terms throughout this file.

Done.

@Scottmitch I'm not sure what the original intent was here, but the initial call to `stream.close()` will deactivate the stream anyway so AFAICT the call to `connection().deactivate(stream)` is a no-op.

+1. leave as is.

Yes good catch...`deactivateInternal` was being called in `close()`.


Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
index bc3f6d8129a..f87e098d27a 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java
@@ -139,11 +139,6 @@ public Set<Http2Stream> activeStreams() {
         return Collections.unmodifiableSet(activeStreams);
     }
 
-    @Override
-    public void deactivate(Http2Stream stream) {
-      deactivateInternal((DefaultStream) stream);
-    }
-
     @Override
     public Endpoint<Http2LocalFlowController> local() {
         return localEndpoint;
@@ -159,16 +154,6 @@ public boolean isGoAway() {
         return goAwaySent() || goAwayReceived();
     }
 
-    @Override
-    public Http2Stream createLocalStream(int streamId) throws Http2Exception {
-        return local().createStream(streamId);
-    }
-
-    @Override
-    public Http2Stream createRemoteStream(int streamId) throws Http2Exception {
-        return remote().createStream(streamId);
-    }
-
     @Override
     public boolean goAwayReceived() {
         return localEndpoint.lastKnownStream >= 0;
@@ -200,33 +185,6 @@ private void removeStream(DefaultStream stream) {
         stream.parent().removeChild(stream);
     }
 
-    private void activateInternal(DefaultStream stream) {
-        if (activeStreams.add(stream)) {
-            // Update the number of active streams initiated by the endpoint.
-            stream.createdBy().numActiveStreams++;
-
-            // Notify the listeners.
-            for (Listener listener : listeners) {
-                listener.streamActive(stream);
-            }
-        }
-    }
-
-    private void deactivateInternal(DefaultStream stream) {
-        if (activeStreams.remove(stream)) {
-            // Update the number of active streams initiated by the endpoint.
-            stream.createdBy().numActiveStreams--;
-
-            // Notify the listeners.
-            for (Listener listener : listeners) {
-                listener.streamInactive(stream);
-            }
-
-            // Mark this stream for removal.
-            removalPolicy.markForRemoval(stream);
-        }
-    }
-
     /**
      * Simple stream implementation. Streams can be compared to each other by priority.
      */
@@ -388,7 +346,15 @@ public Http2Stream open(boolean halfClosed) throws Http2Exception {
                 throw streamError(id, PROTOCOL_ERROR, "Attempting to open a stream in an invalid state: " + state);
             }
 
-            activateInternal(this);
+            if (activeStreams.add(this)) {
+                // Update the number of active streams initiated by the endpoint.
+                createdBy().numActiveStreams++;
+
+                // Notify the listeners.
+                for (Listener listener : listeners) {
+                    listener.streamActive(this);
+                }
+            }
             return this;
         }
 
@@ -399,7 +365,20 @@ public Http2Stream close() {
             }
 
             state = CLOSED;
-            deactivateInternal(this);
+            if (activeStreams.remove(this)) {
+                try {
+                    // Update the number of active streams initiated by the endpoint.
+                    createdBy().numActiveStreams--;
+
+                    // Notify the listeners.
+                    for (Listener listener : listeners) {
+                        listener.streamClosed(this);
+                    }
+                } finally {
+                    // Mark this stream for removal.
+                    removalPolicy.markForRemoval(this);
+                }
+            }
             return this;
         }
 
@@ -691,16 +670,8 @@ private final class DefaultEndpoint<F extends Http2FlowController> implements En
         private int lastKnownStream = -1;
         private boolean pushToAllowed = true;
         private F flowController;
-
-        /**
-         * The maximum number of active streams allowed to be created by this endpoint.
-         */
-        private int maxStreams;
-
-        /**
-         * The current number of active streams created by this endpoint.
-         */
         private int numActiveStreams;
+        private int maxActiveStreams;
 
         DefaultEndpoint(boolean server) {
             this.server = server;
@@ -713,7 +684,7 @@ private final class DefaultEndpoint<F extends Http2FlowController> implements En
 
             // Push is disallowed by default for servers and allowed for clients.
             pushToAllowed = !server;
-            maxStreams = Integer.MAX_VALUE;
+            maxActiveStreams = Integer.MAX_VALUE;
         }
 
         @Override
@@ -730,8 +701,8 @@ public boolean createdStreamId(int streamId) {
         }
 
         @Override
-        public boolean acceptingNewStreams() {
-            return nextStreamId() > 0 && numActiveStreams + 1 <= maxStreams;
+        public boolean canCreateStream() {
+            return nextStreamId() > 0 && numActiveStreams + 1 <= maxActiveStreams;
         }
 
         @Override
@@ -813,13 +784,13 @@ public int numActiveStreams() {
         }
 
         @Override
-        public int maxStreams() {
-            return maxStreams;
+        public int maxActiveStreams() {
+            return maxActiveStreams;
         }
 
         @Override
-        public void maxStreams(int maxStreams) {
-            this.maxStreams = maxStreams;
+        public void maxActiveStreams(int maxActiveStreams) {
+            this.maxActiveStreams = maxActiveStreams;
         }
 
         @Override
@@ -866,7 +837,7 @@ private void checkNewStreamAllowed(int streamId) throws Http2Exception {
                 throw connectionError(PROTOCOL_ERROR, "Cannot create a stream since the connection is going away");
             }
             verifyStreamId(streamId);
-            if (!acceptingNewStreams()) {
+            if (!canCreateStream()) {
                 throw connectionError(REFUSED_STREAM, "Maximum streams exceeded for this endpoint.");
             }
         }
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java
index f2406b2ec15..d96dd9e24f5 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java
@@ -143,7 +143,7 @@ public Http2Settings localSettings() {
         Http2HeaderTable headerTable = config.headerTable();
         Http2FrameSizePolicy frameSizePolicy = config.frameSizePolicy();
         settings.initialWindowSize(flowController().initialWindowSize());
-        settings.maxConcurrentStreams(connection.remote().maxStreams());
+        settings.maxConcurrentStreams(connection.remote().maxActiveStreams());
         settings.headerTableSize(headerTable.maxHeaderTableSize());
         settings.maxFrameSize(frameSizePolicy.maxFrameSize());
         settings.maxHeaderListSize(headerTable.maxHeaderListSize());
@@ -170,7 +170,7 @@ public void localSettings(Http2Settings settings) throws Http2Exception {
         Long maxConcurrentStreams = settings.maxConcurrentStreams();
         if (maxConcurrentStreams != null) {
             int value = (int) Math.min(maxConcurrentStreams, Integer.MAX_VALUE);
-            connection.remote().maxStreams(value);
+            connection.remote().maxActiveStreams(value);
         }
 
         Long headerTableSize = settings.headerTableSize();
@@ -322,7 +322,7 @@ public void onHeadersRead(ChannelHandlerContext ctx, int streamId, Http2Headers
             }
 
             if (stream == null) {
-                stream = connection.createRemoteStream(streamId).open(endOfStream);
+                stream = connection.remote().createStream(streamId).open(endOfStream);
             } else {
                 switch (stream.state()) {
                     case RESERVED_REMOTE:
@@ -371,7 +371,7 @@ public void onPriorityRead(ChannelHandlerContext ctx, int streamId, int streamDe
             if (stream == null) {
                 // PRIORITY frames always identify a stream. This means that if a PRIORITY frame is the
                 // first frame to be received for a stream that we must create the stream.
-                stream = connection.createRemoteStream(streamId);
+                stream = connection.remote().createStream(streamId);
             }
 
             // This call will create a stream for streamDependency if necessary.
@@ -428,7 +428,7 @@ private void applyLocalSettings(Http2Settings settings) throws Http2Exception {
             Long maxConcurrentStreams = settings.maxConcurrentStreams();
             if (maxConcurrentStreams != null) {
                 int value = (int) Math.min(maxConcurrentStreams, Integer.MAX_VALUE);
-                connection.remote().maxStreams(value);
+                connection.remote().maxActiveStreams(value);
             }
 
             Long headerTableSize = settings.headerTableSize();
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
index ebb7a9ebabd..37b723a03bf 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoder.java
@@ -121,7 +121,7 @@ public void remoteSettings(Http2Settings settings) throws Http2Exception {
 
         Long maxConcurrentStreams = settings.maxConcurrentStreams();
         if (maxConcurrentStreams != null) {
-            connection.local().maxStreams((int) Math.min(maxConcurrentStreams, Integer.MAX_VALUE));
+            connection.local().maxActiveStreams((int) Math.min(maxConcurrentStreams, Integer.MAX_VALUE));
         }
 
         Long headerTableSize = settings.headerTableSize();
@@ -194,7 +194,7 @@ public ChannelFuture writeHeaders(final ChannelHandlerContext ctx, final int str
             }
             Http2Stream stream = connection.stream(streamId);
             if (stream == null) {
-                stream = connection.createLocalStream(streamId);
+                stream = connection.local().createStream(streamId);
             }
 
             switch (stream.state()) {
@@ -235,7 +235,7 @@ public ChannelFuture writePriority(ChannelHandlerContext ctx, int streamId, int
             // Update the priority on this stream.
             Http2Stream stream = connection.stream(streamId);
             if (stream == null) {
-                stream = connection.createLocalStream(streamId);
+                stream = connection.local().createStream(streamId);
             }
 
             stream.setPriority(streamDependency, weight, exclusive);
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
index 62420691f51..767cf04678d 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
@@ -73,7 +73,7 @@ public void streamActive(Http2Stream stream) {
             }
 
             @Override
-            public void streamInactive(Http2Stream stream) {
+            public void streamClosed(Http2Stream stream) {
                 // Any pending frames can never be written, cancel and
                 // write errors for any pending frames.
                 state(stream).cancel();
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java
index a4207077368..008fe84c7fe 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Connection.java
@@ -28,26 +28,26 @@ public interface Http2Connection {
     interface Listener {
         /**
          * Notifies the listener that the given stream was added to the connection. This stream may
-         * not yet be active (i.e. open/half-closed).
+         * not yet be active (i.e. {@code OPEN} or {@code HALF CLOSED}).
          */
         void streamAdded(Http2Stream stream);
 
         /**
-         * Notifies the listener that the given stream was made active (i.e. open in at least one
-         * direction).
+         * Notifies the listener that the given stream was made active (i.e. {@code OPEN} or {@code HALF CLOSED}).
          */
         void streamActive(Http2Stream stream);
 
         /**
-         * Notifies the listener that the given stream is now half-closed. The stream can be
-         * inspected to determine which side is closed.
+         * Notifies the listener that the given stream is now {@code HALF CLOSED}. The stream can be
+         * inspected to determine which side is {@code CLOSED}.
          */
         void streamHalfClosed(Http2Stream stream);
 
         /**
-         * Notifies the listener that the given stream is now closed in both directions.
+         * Notifies the listener that the given stream is now {@code CLOSED} in both directions and will no longer
+         * be returned by {@link #activeStreams()}.
          */
-        void streamInactive(Http2Stream stream);
+        void streamClosed(Http2Stream stream);
 
         /**
          * Notifies the listener that the given stream has now been removed from the connection and
@@ -106,18 +106,18 @@ interface Endpoint<F extends Http2FlowController> {
         boolean createdStreamId(int streamId);
 
         /**
-         * Indicates whether or not this endpoint is currently accepting new streams. This will be
-         * be false if {@link #numActiveStreams()} + 1 >= {@link #maxStreams()} or if the stream IDs
+         * Indicates whether or not this endpoint is currently allowed to create new streams. This will be
+         * be false if {@link #numActiveStreams()} + 1 >= {@link #maxActiveStreams()} or if the stream IDs
          * for this endpoint have been exhausted (i.e. {@link #nextStreamId()} < 0).
          */
-        boolean acceptingNewStreams();
+        boolean canCreateStream();
 
         /**
          * Creates a stream initiated by this endpoint. This could fail for the following reasons:
          * <ul>
          * <li>The requested stream ID is not the next sequential ID for this endpoint.</li>
          * <li>The stream already exists.</li>
-         * <li>The number of concurrent streams is above the allowed threshold for this endpoint.</li>
+         * <li>{@link #canCreateStream()} is {@code false}.</li>
          * <li>The connection is marked as going away.</li>
          * </ul>
          * <p>
@@ -135,7 +135,7 @@ interface Endpoint<F extends Http2FlowController> {
          * <li>The requested stream ID is not the next sequential stream ID for this endpoint.</li>
          * <li>The number of concurrent streams is above the allowed threshold for this endpoint.</li>
          * <li>The connection is marked as going away.</li>
-         * <li>The parent stream ID does not exist or is not open from the side sending the push
+         * <li>The parent stream ID does not exist or is not {@code OPEN} from the side sending the push
          * promise.</li>
          * <li>Could not set a valid priority for the new stream.</li>
          * </ul>
@@ -162,19 +162,24 @@ interface Endpoint<F extends Http2FlowController> {
         boolean allowPushTo();
 
         /**
-         * Gets the number of currently active streams that were created by this endpoint.
+         * Gets the number of active streams (i.e. {@code OPEN} or {@code HALF CLOSED}) that were created by this
+         * endpoint.
          */
         int numActiveStreams();
 
         /**
-         * Gets the maximum number of concurrent streams allowed by this endpoint.
+         * Gets the maximum number of streams (created by this endpoint) that are allowed to be active at
+         * the same time. This is the {@code SETTINGS_MAX_CONCURRENT_STREAMS} value sent from the opposite endpoint to
+         * restrict stream creation by this endpoint.
          */
-        int maxStreams();
+        int maxActiveStreams();
 
         /**
-         * Sets the maximum number of concurrent streams allowed by this endpoint.
+         * Sets the maximum number of streams (created by this endpoint) that are allowed to be active at once.
+         * This is the {@code SETTINGS_MAX_CONCURRENT_STREAMS} value sent from the opposite endpoint to
+         * restrict stream creation by this endpoint.
          */
-        void maxStreams(int maxStreams);
+        void maxActiveStreams(int maxActiveStreams);
 
         /**
          * Gets the ID of the stream last successfully created by this endpoint.
@@ -231,26 +236,16 @@ interface Endpoint<F extends Http2FlowController> {
     Http2Stream connectionStream();
 
     /**
-     * Gets the number of streams that actively in use. It is possible for a stream to be closed
-     * but still be considered active (e.g. there is still pending data to be written).
+     * Gets the number of streams that are actively in use (i.e. {@code OPEN} or {@code HALF CLOSED}).
      */
     int numActiveStreams();
 
     /**
-     * Gets all streams that are actively in use. The returned collection is
+     * Gets all streams that are actively in use (i.e. {@code OPEN} or {@code HALF CLOSED}). The returned collection is
      * sorted by priority.
      */
     Collection<Http2Stream> activeStreams();
 
-    /**
-     * Indicates that the given stream is no longer actively in use. If this stream was active,
-     * after calling this method it will no longer appear in the list returned by
-     * {@link #activeStreams()} and {@link #numActiveStreams()} will be decremented. In addition,
-     * all listeners will be notified of this event via
-     * {@link Listener#streamInactive(Http2Stream)}.
-     */
-    void deactivate(Http2Stream stream);
-
     /**
      * Indicates whether or not the local endpoint for this connection is the server.
      */
@@ -261,23 +256,11 @@ interface Endpoint<F extends Http2FlowController> {
      */
     Endpoint<Http2LocalFlowController> local();
 
-    /**
-     * Creates a new stream initiated by the local endpoint
-     * @see Endpoint#createStream(int)
-     */
-    Http2Stream createLocalStream(int streamId) throws Http2Exception;
-
     /**
      * Gets a view of this connection from the remote {@link Endpoint}.
      */
     Endpoint<Http2RemoteFlowController> remote();
 
-    /**
-     * Creates a new stream initiated by the remote endpoint.
-     * @see Endpoint#createStream(int)
-     */
-    Http2Stream createRemoteStream(int streamId) throws Http2Exception;
-
     /**
      * Indicates whether or not a {@code GOAWAY} was received from the remote endpoint.
      */
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionAdapter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionAdapter.java
index 5ac64ee2e09..b6b878d6364 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionAdapter.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionAdapter.java
@@ -32,7 +32,7 @@ public void streamHalfClosed(Http2Stream stream) {
     }
 
     @Override
-    public void streamInactive(Http2Stream stream) {
+    public void streamClosed(Http2Stream stream) {
     }
 
     @Override
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 f5fc5dc6fd6..1b4e974786e 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
@@ -125,7 +125,7 @@ public void onHttpClientUpgrade() throws Http2Exception {
         }
 
         // Create a local stream used for the HTTP cleartext upgrade.
-        connection().createLocalStream(HTTP_UPGRADE_STREAM_ID).open(true);
+        connection().local().createStream(HTTP_UPGRADE_STREAM_ID).open(true);
     }
 
     /**
@@ -144,7 +144,7 @@ public void onHttpServerUpgrade(Http2Settings settings) throws Http2Exception {
         encoder.remoteSettings(settings);
 
         // Create a stream in the half-closed state.
-        connection().createRemoteStream(HTTP_UPGRADE_STREAM_ID).open(true);
+        connection().remote().createStream(HTTP_UPGRADE_STREAM_ID).open(true);
     }
 
     @Override
@@ -262,9 +262,6 @@ public void closeStream(final Http2Stream stream, ChannelFuture future) {
         future.addListener(new ChannelFutureListener() {
           @Override
           public void operationComplete(ChannelFuture future) throws Exception {
-            // Deactivate this stream.
-            connection().deactivate(stream);
-
             // If this connection is closing and there are no longer any
             // active streams, close after the current operation completes.
             if (closeListener != null && connection().numActiveStreams() == 0) {
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EventAdapter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EventAdapter.java
index 002b36817fc..30a3f9a18a2 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EventAdapter.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2EventAdapter.java
@@ -96,7 +96,7 @@ public void streamHalfClosed(Http2Stream stream) {
     }
 
     @Override
-    public void streamInactive(Http2Stream stream) {
+    public void streamClosed(Http2Stream stream) {
     }
 
     @Override


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
index 0737811c378..5f1fa909438 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoderTest.java
@@ -128,20 +128,6 @@ public void setup() throws Exception {
         when(local.flowController()).thenReturn(localFlow);
         when(encoder.flowController()).thenReturn(remoteFlow);
         when(connection.remote()).thenReturn(remote);
-        doAnswer(new Answer<Http2Stream>() {
-            @Override
-            public Http2Stream answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                return local.createStream((Integer) args[0]);
-            }
-        }).when(connection).createLocalStream(anyInt());
-        doAnswer(new Answer<Http2Stream>() {
-            @Override
-            public Http2Stream answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                return remote.createStream((Integer) args[0]);
-            }
-        }).when(connection).createRemoteStream(anyInt());
         when(local.createStream(eq(STREAM_ID))).thenReturn(stream);
         when(local.reservePushStream(eq(PUSH_STREAM_ID), eq(stream))).thenReturn(pushStream);
         when(remote.createStream(eq(STREAM_ID))).thenReturn(stream);
@@ -389,7 +375,7 @@ public void priorityReadShouldSucceed() throws Exception {
         decode().onPriorityRead(ctx, STREAM_ID, 0, (short) 255, true);
         verify(stream).setPriority(eq(0), eq((short) 255), eq(true));
         verify(listener).onPriorityRead(eq(ctx), eq(STREAM_ID), eq(0), eq((short) 255), eq(true));
-        verify(connection).createRemoteStream(STREAM_ID);
+        verify(remote).createStream(STREAM_ID);
         verify(stream, never()).open(anyBoolean());
     }
 
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
index 45ccc991208..b526f473dff 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionEncoderTest.java
@@ -18,10 +18,10 @@
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGHT;
 import static io.netty.handler.codec.http2.Http2CodecUtil.emptyPingBuf;
 import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
+import static io.netty.handler.codec.http2.Http2Stream.State.HALF_CLOSED_LOCAL;
 import static io.netty.handler.codec.http2.Http2Stream.State.IDLE;
 import static io.netty.handler.codec.http2.Http2Stream.State.OPEN;
 import static io.netty.handler.codec.http2.Http2Stream.State.RESERVED_LOCAL;
-import static io.netty.handler.codec.http2.Http2Stream.State.HALF_CLOSED_LOCAL;
 import static io.netty.util.CharsetUtil.UTF_8;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -36,7 +36,6 @@
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -51,7 +50,6 @@
 import io.netty.channel.DefaultChannelPromise;
 import io.netty.handler.codec.http2.Http2RemoteFlowController.FlowControlled;
 import io.netty.util.concurrent.ImmediateEventExecutor;
-
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -142,20 +140,6 @@ public void setup() throws Exception {
         when(writer.configuration()).thenReturn(writerConfig);
         when(writerConfig.frameSizePolicy()).thenReturn(frameSizePolicy);
         when(frameSizePolicy.maxFrameSize()).thenReturn(64);
-        doAnswer(new Answer<Http2Stream>() {
-            @Override
-            public Http2Stream answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                return local.createStream((Integer) args[0]);
-            }
-        }).when(connection).createLocalStream(anyInt());
-        doAnswer(new Answer<Http2Stream>() {
-            @Override
-            public Http2Stream answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                return remote.createStream((Integer) args[0]);
-            }
-        }).when(connection).createRemoteStream(anyInt());
         when(local.createStream(eq(STREAM_ID))).thenReturn(stream);
         when(local.reservePushStream(eq(PUSH_STREAM_ID), eq(stream))).thenReturn(pushStream);
         when(remote.createStream(eq(STREAM_ID))).thenReturn(stream);
@@ -389,7 +373,7 @@ public void priorityWriteShouldSetPriorityForStream() throws Exception {
         encoder.writePriority(ctx, STREAM_ID, 0, (short) 255, true, promise);
         verify(stream).setPriority(eq(0), eq((short) 255), eq(true));
         verify(writer).writePriority(eq(ctx), eq(STREAM_ID), eq(0), eq((short) 255), eq(true), eq(promise));
-        verify(connection).createLocalStream(STREAM_ID);
+        verify(local).createStream(STREAM_ID);
         verify(stream, never()).open(anyBoolean());
     }
 
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java
index 5e394104b48..18e0ba98813 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionTest.java
@@ -163,7 +163,7 @@ public void newStreamNotForClientShouldThrow() throws Http2Exception {
 
     @Test(expected = Http2Exception.class)
     public void maxAllowedStreamsExceededShouldThrow() throws Http2Exception {
-        server.local().maxStreams(0);
+        server.local().maxActiveStreams(0);
         server.local().createStream(2).open(true);
     }
 
@@ -214,12 +214,12 @@ public void closeOnlyOpenSideShouldClose() throws Http2Exception {
 
     @Test(expected = Http2Exception.class)
     public void localStreamInvalidStreamIdShouldThrow() throws Http2Exception {
-        client.createLocalStream(Integer.MAX_VALUE + 2).open(false);
+        client.local().createStream(Integer.MAX_VALUE + 2).open(false);
     }
 
     @Test(expected = Http2Exception.class)
     public void remoteStreamInvalidStreamIdShouldThrow() throws Http2Exception {
-        client.createRemoteStream(Integer.MAX_VALUE + 1).open(false);
+        client.remote().createStream(Integer.MAX_VALUE + 1).open(false);
     }
 
     @Test
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
index 190c837391a..ea9844451cd 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
@@ -111,20 +111,6 @@ public void setup() throws Exception {
         when(connection.local()).thenReturn(local);
         when(connection.activeStreams()).thenReturn(Collections.singletonList(stream));
         when(stream.open(anyBoolean())).thenReturn(stream);
-        doAnswer(new Answer<Http2Stream>() {
-            @Override
-            public Http2Stream answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                return local.createStream((Integer) args[0]);
-            }
-        }).when(connection).createLocalStream(anyInt());
-        doAnswer(new Answer<Http2Stream>() {
-            @Override
-            public Http2Stream answer(InvocationOnMock invocation) throws Throwable {
-                Object[] args = invocation.getArguments();
-                return remote.createStream((Integer) args[0]);
-            }
-        }).when(connection).createRemoteStream(anyInt());
         when(encoder.writeSettings(eq(ctx), any(Http2Settings.class), eq(promise))).thenReturn(future);
         when(ctx.alloc()).thenReturn(UnpooledByteBufAllocator.DEFAULT);
         when(ctx.channel()).thenReturn(channel);