Repository: netty/netty
File: codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java (Java)

Code before:
                    .stream(requireStream(streamId)));
        }

        @Override
        public int onDataRead(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding,
                              boolean endOfStream) {
            onHttp2Frame(ctx, new DefaultHttp2DataFrame(data, endOfStream, padding)
                    .stream(requireStream(streamId)).retain());
            // We return the bytes in consumeBytes() once the stream channel consumed the bytes.
            return 0;
        }

        @Override
        public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData) {
            onHttp2Frame(ctx, new DefaultHttp2GoAwayFrame(lastStreamId, errorCode, debugData).retain());
        }

        @Override
        public void onPriorityRead(ChannelHandlerContext ctx, int streamId, int streamDependency,
                                   short weight, boolean exclusive) {

            Http2Stream stream = connection().stream(streamId);
            if (stream == null) {
                // The stream was not opened yet, let's just ignore this for now.
                return;
            }
            onHttp2Frame(ctx, new DefaultHttp2PriorityFrame(streamDependency, weight, exclusive)
                    .stream(requireStream(streamId)));
        }

        @Override
        public void onSettingsAckRead(ChannelHandlerContext ctx) {
            onHttp2Frame(ctx, Http2SettingsAckFrame.INSTANCE);
        }

        @Override
        public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promisedStreamId,
                                      Http2Headers headers, int padding) {
            onHttp2Frame(ctx, new DefaultHttp2PushPromiseFrame(headers, padding, promisedStreamId)
                    .pushStream(new DefaultHttp2FrameStream()
                            .setStreamAndProperty(streamKey, connection().stream(promisedStreamId)))
                    .stream(requireStream(streamId)));
        }

        private Http2FrameStream requireStream(int streamId) {
            Http2FrameStream stream = connection().stream(streamId).getProperty(streamKey);
            if (stream == null) {
                throw new IllegalStateException("Stream object required for identifier: " + streamId);
            }
            return stream;
        }

Review comment (bug):
As noted above, I think the pattern of building the frame and then once we're ready to forward it calling `.retain()` was better as it didn't retain until everything else was done. With this change if the constructor throws (perhaps a constructor assert added in the future) we leak the `debugData` buffer.

Code after:
                                  int padding, boolean endOfStream) {
            Http2FrameStream stream = requireStream(streamId);
            onHttp2Frame(ctx, new DefaultHttp2HeadersFrame(headers, endOfStream, padding).stream(stream));
        }

        @Override
        public int onDataRead(ChannelHandlerContext ctx, int streamId, ByteBuf data, int padding,
                              boolean endOfStream) {
            Http2FrameStream stream = requireStream(streamId);
            final Http2DataFrame dataframe;
            try {
                dataframe = new DefaultHttp2DataFrame(data.retain(), endOfStream, padding);
            } catch (IllegalArgumentException e) {
                // Might be thrown in case of invalid padding / length.
                data.release();
                throw e;
            }
            dataframe.stream(stream);
            onHttp2Frame(ctx, dataframe);
            // We return the bytes in consumeBytes() once the stream channel consumed the bytes.
            return 0;
        }

        @Override
        public void onGoAwayRead(ChannelHandlerContext ctx, int lastStreamId, long errorCode, ByteBuf debugData) {
            onHttp2Frame(ctx, new DefaultHttp2GoAwayFrame(lastStreamId, errorCode, debugData.retain()));
        }

        @Override
        public void onPriorityRead(ChannelHandlerContext ctx, int streamId, int streamDependency,
                                   short weight, boolean exclusive) {

            Http2Stream stream = connection().stream(streamId);
            if (stream == null) {
                // The stream was not opened yet, let's just ignore this for now.
                return;
            }
            Http2FrameStream frameStream = requireStream(streamId);
            onHttp2Frame(ctx, new DefaultHttp2PriorityFrame(streamDependency, weight, exclusive)
                    .stream(frameStream));
        }

        @Override
        public void onSettingsAckRead(ChannelHandlerContext ctx) {
            onHttp2Frame(ctx, Http2SettingsAckFrame.INSTANCE);
        }

        @Override
        public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promisedStreamId,
                                      Http2Headers headers, int padding) {
            Http2FrameStream stream = requireStream(streamId);