Repository: netty/netty
Issue: Get rid of Http2StreamRemovalPolicy
Labels: improvement

The `DefaultHttp2StreamRemovalPolicy` by default waits 5 seconds after a stream has been closed before removing it from the connection.  The reason for this is to support the following case from the HTTP2 spec (From [Section 5.1](https://tools.ietf.org/html/draft-ietf-httpbis-http2-17#section-5.1)):

```
"WINDOW_UPDATE or RST_STREAM frames can be received in this state
 for a short period after a DATA or HEADERS frame containing an
 END_STREAM flag is sent.  Until the remote peer receives and
 processes RST_STREAM or the frame bearing the END_STREAM flag, it
 might send frames of these types.  Endpoints MUST ignore
 WINDOW_UPDATE or RST_STREAM frames received in this state, though
 endpoints MAY choose to treat frames that arrive a significant
 time after sending END_STREAM as a connection error
 of type PROTOCOL_ERROR."
```

These cases don't really seem to warrant the extra complexity that DefaultHttp2StreamRemovalPolicy introduces.  It would be better to just always ignore WINDOW_UPDATE and RST_STREAM for non-existent streams.  This way we can always remove the streams as soon as they're closed to avoid potential memory leaks.


Issue comments:
@Scottmitch WDYT?

We ignore these frames for non-existent streams if they are for stream Ids below the lastGoodStreamId

That seems like a reasonable default policy. 

I guess the question here is whether we need to continue to allow for a pluggable policy or just bake this behavior in. 

@jpinner - What is `lastGoodStreamId`?  Does this relate to the stream id from a [GOAWAY](https://tools.ietf.org/html/draft-ietf-httpbis-http2-17#section-6.8)?

@nmittler - I would like to understand the implications of this statement:

> This way we can always remove the streams as soon as they're closed to avoid potential memory leaks.

Does `DefaultHttp2StreamRemovalPolicy` itself have a problem, or the way the interfaces are designed make memory leaks likely?

My first thought are at a minimum we should do some re-naming because the [immediateRemovalPolicy](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java#L119) is really the [default](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L69).  My next thoughts are I have not yet had a need for `DefaultHttp2StreamRemovalPolicy` or to use anything but the  [immediateRemovalPolicy](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java#L119).  I don't necessarily have a problem with the `Http2StreamRemovalPolicy` interface but if we are trying to simplify perhaps a better extension point would be to make [DefaultHttp2Connection.deactivateInternal](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L226)'s call to `removalPolicy.markForRemoval(stream);` a bit more accessible?  Something like adding a method to DefaultHttp2Connection that is called instead of using the `removalPolicy`:

``` java
protected void markForRemoval(Http2Stream stream) {
  removeStream(stream);
}
```

We could then get rid of `Http2StreamRemovalPolicy`.  Less object creation/garbage, conditional checking, method call redirection, and supporting interfaces.  WDYT?

@Scottmitch sorry, should have been more explicit -- yes, it's the stream id that would go in a goaway frame if you were to start shutting down the connection

@nmittler - I re-read your issue description and looked at the interfaces a bit more...I think I came to the same conclusion as you did :)

Just call [DefaultHttp2Connection.removeStream](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L192) from [DefaultHttp2Connection.deactivateInternal](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L226).  Consider making [DefaultHttp2Connection.removeStream](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L192) `protected` if we want to preserve the "removal" hooks.  Is this what you were originally suggesting?

@jpinner - Thanks for clarifying, and this makes sense.

@nmittler - Could we just use [connection.remote().lastStreamCreated()](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java#L377) for this purpose?  I'm hesitant because the application has the ability to send a go away with what ever stream id they want.  We may need to consider providing an extension point which returns `connection.remote().lastStreamCreated()` by default.  This would give the application the opportunity to override our somewhat speculative question "if we were to send a go away now, what stream id would the application want to use".

@Scottmitch I think we should just get rid of everything related to stream "removal" and just have "closed" be the end state.

I think for now we can just use `connection.remove().lastStreamCreated()` ... this would maintain the current behavior.  We can look for a way of allowing the application to override this in the future, but I think that's beyond the scope here.  WDYT?

@nmittler - I think we are mostly in agreement and we can work out the details in the PR.

> I think for now we can just use connection.remove().lastStreamCreated() ... this would maintain the current behavior. 

This is not obvious to me yet. [onWindowUpdateRead](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java#L531) and [onRstStreamRead](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java#L388) both use `connection.requireStream(...)`.  This will change to support the ignore "frames for streams that don't exist but are lower than the stream id that would be sent in a go away frame".  The user currently has some control over when errors get returned in these situations via the "removal" mechanism.  If we force `connection.remote().lastStreamCreated()` we may be taking this control away.  I think this is a good default but the question is should we be providing an extension point as we introduce these changes.

@Scottmitch agreed, that's a good point.  Do you have any thoughts on what the API might look like?

Hmm I can't really dig too deep yet but here is an option:

Add something like `int lastGoodStream()` to `Http2Connection.EndPoint` or `Http2Connection`. Then either:
1. just return `connection.remote().lastKnownStream()` from the default classes (extension point is  extend the class and override method). OR;
2. create an interface which can be extended (see below) and call this interface to do the work for you.

``` java
public interface Http2StreamStatus {
    public static final Http2RemoteStreamStatus DEFAULT = new Http2RemoteStreamStatus() {
        @Override
        public int lastGoodStreamId(Http2Connection connection) {
            return connection.remote().lastKnownStream();
        }
    };

    int lastGoodStreamId(Http2Connection connection);
}
```

Thoughts? Can you come up with anything better?

We should also consider how to reconcile any potentially new interfaces with the stream id parameter on `Http2LifecycleManager.writeGoAway`, `Http2ConnectionHandler.writeGoAway`, `DefaultHttp2ConnectionEncoder.writeGoAway`, etc... Should we not allow a stream ID here, and just use the stream ID from this new interface?  Is there a more straightforward way to present this?

That seems reasonable.  I might suggest some name changes (this can probably be improved as well):

``` java
public interface Http2GoAwayProvider {
    public static final Http2GoAwayProvider DEFAULT = new Http2GoAwayProvider() {
        @Override
        public int goAwayStream(Http2Connection connection) {
            return connection.remote().lastKnownStream();
        }
    };

    int goAwayStream(Http2Connection connection);
}
```

@nmittler - SGTM.  Should have added a caveat: names are for demo purposes only...use at your own risk ;)

Ha! We should have that disclaimer on all of our .java files :)

@Scottmitch I'm just getting back to this.  I think in general, we can get away with removing streams as we close them. In all the inbound frame handler methods, we could more-or-less ignore frames for streams that have been closed (i.e. existing-and-closed or the streamId <= `lastKnownStream` created by either `Endpoint`).

We could be more picky and maintain some history of streamIds for which we've sent `RST_STREAM` in either direction and trigger errors if an `Endpoint` sends a (disallowed) frame after it had already sent `RST_STREAM`.  However, I'm not sure it would be worth it - it might be simpler and with less overhead to just to ignore all frames (to the extent possible) for any historical stream (existing-and-closed or possibly-existed).

The only caveat to this (that I can think of ATM) would be the priority tree for outbound flow control.  I believe we need to keep any closed streams around so long as they still have children.  Once all of their children are removed, the closed stream can then be removed from the connection entirely. Actually now that I'm thinking about it, this might be a separate task unto itself since this would currently be a problem if using the default (immediate) stream removal policy.  We should fix this to delay stream removal until after all dependencies are removed.

WDYT?

> In all the inbound frame handler methods, we could more-or-less ignore frames for streams that have been closed (i.e. existing-and-closed or the streamId <= lastKnownStream created by either Endpoint).

+1. Yes seems reasonable to me.

> We could be more picky and maintain some history of streamIds for which we've sent RST_STREAM in either direction and trigger errors if an Endpoint sends a (disallowed) frame after it had already sent RST_STREAM

Perhaps we don't maintain history by default....but it may be worth it to provide an interface which supports this?  Kinda like we have today....humm

> The only caveat to this (that I can think of ATM) would be the priority tree for outbound flow control.

Yes this may be problematic.  Lets handle this as a separate issue.  I think we need to think about this because `WINDOW_UPDATES` are only accepted for a `short period after a DATA or HEADERS frame containing an END_STREAM flag is sent` while PRIORITY frames can be received indefinitely.  I'm wondering if some kind of deadlock may occur if we keep something in the priority tree and the window cannot be adjusted....

> I'm wondering if some kind of deadlock may occur if we keep something in the priority tree and the window cannot be adjusted....

It seems reasonable to me that we always except WINDOW_UPDATES for streams that are still in the priority tree.  Assume stream A is closed and has children B and C. If we receive a `WINDOW_UPDATE` for A, I would expect it to still impact the data written for B and C. After  B and C are closed (and removed), we can then remove A. From that point forward we ignore WINDOW_UPDATES for stream A.

> It seems reasonable to me that we always except WINDOW_UPDATES for streams that are still in the priority tree.

I agree.  However the specification may not allow sending of WINDOW_UPDATES in these states.  Just need to think about this I guess....

Created new issue for the priority tree management https://github.com/netty/netty/issues/3518.

ahh there is already an issue for this, I was thinking about opening one myself this afternoon :-). +1

@nmittler - Is this still on your plate?

@Scottmitch I'm was waiting for the dust to settle on some of the other changes to flow control.  I think everything is in now so I can take a look ... or were you looking to take this on?

@nmittler - Nope I haven't touched this.

@nmittler - Do you want https://github.com/netty/netty/pull/3543 to be pulled in before this?

@Scottmitch yeah I think that makes sense.

@nmittler - #3543 has been cherry-picked...you should now be unblocked.

Nice to see this being fixed! We have seen some very large object allocations (several megabytes per object) due to the removal policy i.e. when opening and closing ~20,000 streams per second.

@buchgr - Cool to see numbers and that you are pushing the codec hard...not cool that they appear to be less then optimal :)  I think we are in a better place to tackle this and I hope it simplifies things and helps with the number too.  Have you tried using the ImmediateRemovalPolicy instead?  Do you see any other areas for improvement that we are not currently working on?

@nmittler - A few PRs have been cherry-picked since we last visited this.  Let me know if this is still blocked, or you are too busy to handle this.  I would like to get this in before https://github.com/netty/netty/issues/3501.

@Scottmitch I talked to @nmittler and I ll submit a PR for this.


Pull request review comments:
Kind of strange to say "Default implementation" as this is not an implementation of an interface.

@nmittler I think this comment is not right. Streams should be removed even if they have children, but the children should be reprioritised and get a new parent?

@buchgr I believe the comment accurately reflects the current behavior

@Scottmitch any thoughts?

@nmittler hmm okay, would this then not allow for leaks i.e. "closed parent streams" sticking around after all their children have been removed?

+1. I removed the interface and just kept the class as part of review.  A comment similar to what you added for `interface Event` may make more sense?

> @buchgr I believe the comment accurately reflects the current behavior

+1.

The current behavior is such that streams remain in the priority tree until `prioritizableForTree() == 0`.  Events which can change this count (parent change, stream removal from tree, state change, etc...) re-evaluate whether the stream still belongs in the tree, and calls `removeStream` if not.

@nmittler - I'm not sure we need this comment here though...`removeStream` has a pretty detailed comment on it...maybe just clean that comment up if it is not adequite?

> The current behavior is such that streams remain in the priority tree until prioritizableForTree() == 0. Events which can change this count (parent change, stream removal from tree, state change, etc...) re-evaluate whether the stream still belongs in the tree, and calls removeStream if not.

nice. thanks for clarifying!

Done.

Comment removed.


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 a83436b6493..7c2fe828643 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
@@ -19,7 +19,6 @@
 import static io.netty.handler.codec.http2.Http2CodecUtil.DEFAULT_PRIORITY_WEIGHT;
 import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_WEIGHT;
 import static io.netty.handler.codec.http2.Http2CodecUtil.MIN_WEIGHT;
-import static io.netty.handler.codec.http2.Http2CodecUtil.immediateRemovalPolicy;
 import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
 import static io.netty.handler.codec.http2.Http2Error.REFUSED_STREAM;
 import static io.netty.handler.codec.http2.Http2Exception.closedStreamError;
@@ -35,7 +34,6 @@
 import static io.netty.util.internal.ObjectUtil.checkNotNull;
 
 import io.netty.buffer.ByteBuf;
-import io.netty.handler.codec.http2.Http2StreamRemovalPolicy.Action;
 import io.netty.util.collection.IntObjectHashMap;
 import io.netty.util.collection.IntObjectMap;
 import io.netty.util.collection.PrimitiveCollections;
@@ -70,37 +68,17 @@ public class DefaultHttp2Connection implements Http2Connection {
     final List<Listener> listeners = new ArrayList<Listener>(4);
     final ActiveStreams activeStreams;
 
-    /**
-     * Creates a connection with an immediate stream removal policy.
-     *
-     * @param server
-     *            whether or not this end-point is the server-side of the HTTP/2 connection.
-     */
-    public DefaultHttp2Connection(boolean server) {
-        this(server, immediateRemovalPolicy());
-    }
-
     /**
      * Creates a new connection with the given settings.
      *
      * @param server
      *            whether or not this end-point is the server-side of the HTTP/2 connection.
-     * @param removalPolicy
-     *            the policy to be used for removal of closed stream.
      */
-    public DefaultHttp2Connection(boolean server, Http2StreamRemovalPolicy removalPolicy) {
-        activeStreams = new ActiveStreams(listeners, checkNotNull(removalPolicy, "removalPolicy"));
+    public DefaultHttp2Connection(boolean server) {
+        activeStreams = new ActiveStreams(listeners);
         localEndpoint = new DefaultEndpoint<Http2LocalFlowController>(server);
         remoteEndpoint = new DefaultEndpoint<Http2RemoteFlowController>(!server);
 
-        // Tell the removal policy how to remove a stream from this connection.
-        removalPolicy.setAction(new Action() {
-            @Override
-            public void removeStream(Http2Stream stream) {
-                DefaultHttp2Connection.this.removeStream((DefaultStream) stream);
-            }
-        });
-
         // Add the connection stream to the map.
         streamMap.put(connectionStream.id(), connectionStream);
     }
@@ -997,31 +975,32 @@ private boolean isLocal() {
     }
 
     /**
-     * Default implementation of the {@link ActiveStreams} class.
+     * Allows events which would modify the collection of active streams to be queued while iterating via {@link
+     * #forEachActiveStream(Http2StreamVisitor)}.
      */
-    private static final class ActiveStreams {
+    interface Event {
         /**
-         * Allows events which would modify {@link #streams} to be queued while iterating over {@link #streams}.
+         * Trigger the original intention of this event. Expect to modify the active streams list.
+         * <p/>
+         * If a {@link RuntimeException} object is thrown it will be logged and <strong>not propagated</strong>.
+         * Throwing from this method is not supported and is considered a programming error.
          */
-        interface Event {
-            /**
-             * Trigger the original intention of this event. Expect to modify {@link #streams}.
-             * <p>
-             * If a {@link RuntimeException} object is thrown it will be logged and <strong>not propagated</strong>.
-             * Throwing from this method is not supported and is considered a programming error.
-             */
-            void process();
-        }
+        void process();
+    }
+
+    /**
+     * Manages the list of currently active streams.  Queues any {@link Event}s that would modify the list of
+     * active streams in order to prevent modification while iterating.
+     */
+    private final class ActiveStreams {
 
         private final List<Listener> listeners;
-        private final Http2StreamRemovalPolicy removalPolicy;
         private final Queue<Event> pendingEvents = new ArrayDeque<Event>(4);
         private final Set<Http2Stream> streams = new LinkedHashSet<Http2Stream>();
         private int pendingIterations;
 
-        public ActiveStreams(List<Listener> listeners, Http2StreamRemovalPolicy removalPolicy) {
+        public ActiveStreams(List<Listener> listeners) {
             this.listeners = listeners;
-            this.removalPolicy = removalPolicy;
         }
 
         public int size() {
@@ -1110,8 +1089,7 @@ void removeFromActiveStreams(DefaultStream stream) {
                         }
                     }
                 } finally {
-                    // Mark this stream for removal.
-                    removalPolicy.markForRemoval(stream);
+                    removeStream(stream);
                 }
             }
         }
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2StreamRemovalPolicy.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2StreamRemovalPolicy.java
deleted file mode 100644
index 550bf4c27a3..00000000000
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2StreamRemovalPolicy.java
+++ /dev/null
@@ -1,106 +0,0 @@
-/*
- * Copyright 2014 The Netty Project
- *
- * The Netty Project licenses this file to you under the Apache License, version 2.0 (the
- * "License"); you may not use this file except in compliance with the License. You may obtain a
- * copy of the License at:
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
- * or implied. See the License for the specific language governing permissions and limitations under
- * the License.
- */
-package io.netty.handler.codec.http2;
-
-import static java.util.concurrent.TimeUnit.NANOSECONDS;
-import static java.util.concurrent.TimeUnit.SECONDS;
-import io.netty.channel.ChannelHandlerAdapter;
-import io.netty.channel.ChannelHandlerContext;
-
-import java.util.ArrayDeque;
-import java.util.Queue;
-import java.util.concurrent.ScheduledFuture;
-
-/**
- * A {@link Http2StreamRemovalPolicy} that periodically runs garbage collection on streams that have
- * been marked for removal.
- */
-public class DefaultHttp2StreamRemovalPolicy extends ChannelHandlerAdapter implements
-        Http2StreamRemovalPolicy, Runnable {
-
-    /**
-     * The interval (in ns) at which the removed priority garbage collector runs.
-     */
-    private static final long GARBAGE_COLLECTION_INTERVAL = SECONDS.toNanos(5);
-
-    private final Queue<Garbage> garbage = new ArrayDeque<Garbage>();
-    private ScheduledFuture<?> timerFuture;
-    private Action action;
-
-    @Override
-    public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
-        // Schedule the periodic timer for performing the policy check.
-        timerFuture = ctx.channel().eventLoop().scheduleWithFixedDelay(this,
-                GARBAGE_COLLECTION_INTERVAL,
-                GARBAGE_COLLECTION_INTERVAL,
-                NANOSECONDS);
-    }
-
-    @Override
-    public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
-        // Cancel the periodic timer.
-        if (timerFuture != null) {
-            timerFuture.cancel(false);
-            timerFuture = null;
-        }
-    }
-
-    @Override
-    public void setAction(Action action) {
-        this.action = action;
-    }
-
-    @Override
-    public void markForRemoval(Http2Stream stream) {
-        garbage.add(new Garbage(stream));
-    }
-
-    /**
-     * Runs garbage collection of any streams marked for removal >
-     * {@link #GARBAGE_COLLECTION_INTERVAL} nanoseconds ago.
-     */
-    @Override
-    public void run() {
-        if (garbage.isEmpty() || action == null) {
-            return;
-        }
-
-        long time = System.nanoTime();
-        for (;;) {
-            Garbage next = garbage.peek();
-            if (next == null) {
-                break;
-            }
-            if (time - next.removalTime > GARBAGE_COLLECTION_INTERVAL) {
-                garbage.remove();
-                action.removeStream(next.stream);
-            } else {
-                break;
-            }
-        }
-    }
-
-    /**
-     * Wrapper around a stream and its removal time.
-     */
-    private static final class Garbage {
-        private final long removalTime = System.nanoTime();
-        private final Http2Stream stream;
-
-        Garbage(Http2Stream stream) {
-            this.stream = stream;
-        }
-    }
-}
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java
index 19b27f59382..87735a53268 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java
@@ -16,14 +16,13 @@
 package io.netty.handler.codec.http2;
 
 import static io.netty.util.CharsetUtil.UTF_8;
-import static io.netty.util.internal.ObjectUtil.checkNotNull;
+
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.DefaultChannelPromise;
-import io.netty.handler.codec.http2.Http2StreamRemovalPolicy.Action;
 import io.netty.util.concurrent.EventExecutor;
 
 /**
@@ -113,30 +112,6 @@ public static ByteBuf emptyPingBuf() {
         return Unpooled.wrappedBuffer(EMPTY_PING);
     }
 
-    /**
-     * Returns a simple {@link Http2StreamRemovalPolicy} that immediately calls back the
-     * {@link Action} when a stream is marked for removal.
-     */
-    public static Http2StreamRemovalPolicy immediateRemovalPolicy() {
-        return new Http2StreamRemovalPolicy() {
-            private Action action;
-
-            @Override
-            public void setAction(Action action) {
-                this.action = checkNotNull(action, "action");
-            }
-
-            @Override
-            public void markForRemoval(Http2Stream stream) {
-                if (action == null) {
-                    throw new IllegalStateException(
-                            "Action must be called before removing streams.");
-                }
-                action.removeStream(stream);
-            }
-        };
-    }
-
     /**
      * Iteratively looks through the causaility chain for the given exception and returns the first
      * {@link Http2Exception} or {@code null} if none.
@@ -321,15 +296,5 @@ public boolean trySuccess(Void result) {
         }
     }
 
-    /**
-     * Fails the given promise with the cause and then re-throws the cause.
-     */
-    public static <T extends Throwable> T failAndThrow(ChannelPromise promise, T cause) throws T {
-        if (!promise.isDone()) {
-            promise.setFailure(cause);
-        }
-        throw cause;
-    }
-
     private Http2CodecUtil() { }
 }
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamRemovalPolicy.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamRemovalPolicy.java
deleted file mode 100644
index d8e57dd7f79..00000000000
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamRemovalPolicy.java
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * Copyright 2014 The Netty Project
- *
- * The Netty Project licenses this file to you under the Apache License, version 2.0 (the
- * "License"); you may not use this file except in compliance with the License. You may obtain a
- * copy of the License at:
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
- * or implied. See the License for the specific language governing permissions and limitations under
- * the License.
- */
-package io.netty.handler.codec.http2;
-
-/**
- * A policy for determining when it is appropriate to remove streams from an HTTP/2 stream registry.
- */
-public interface Http2StreamRemovalPolicy {
-
-    /**
-     * Performs the action of removing the stream.
-     */
-    interface Action {
-        /**
-         * Removes the stream from the registry.
-         */
-        void removeStream(Http2Stream stream);
-    }
-
-    /**
-     * Sets the removal action.
-     */
-    void setAction(Action action);
-
-    /**
-     * Marks the given stream for removal. When this policy has determined that the given stream
-     * should be removed, it will call back the {@link Action}.
-     */
-    void markForRemoval(Http2Stream stream);
-}


Test patch: