feedback-to-code/mozzarella huggingface dataset:
Repository: netty/netty
Issue: Reconsider growing strategy in HpackHuffmanDecoder
Labels: improvement

At the moment we just increase my intialCapacity but it may make sense to double it up to a certain size:

https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackHuffmanDecoder.java#L237

Issue comments:
thanks for reporting @normanmaurer! My guess is this will have a measurable impact if the initial size is too small (currently defaults to 32). We reset to the initial array size on each decode operation.
@Scottmitch agree... I would say we should double up to 512 bytes . WDYT ? I can do a pr
It will depend upon the use case but can we derive a guess from https://github.com/http2/http_samples or some other sample size? We can start with 512 or 1024 and refine later if you don't have much cycles to investigate.
@Scottmitch good idea... let me check
@Scottmitch here you go https://github.com/netty/netty/pull/6857. I think I will have not much time to long into it in more detail so I just used 1024 for now
1024 sounds like a good start to me ... we can revise/improve later if necessary.
Fixed by https://github.com/netty/netty/pull/6857

Pull request review comments:
nit: consider using tertiary for less boiler plate:

```java
final int newLength = bytes.length >= 1024 ? bytes.length + initialCapacity : bytes.length << 1;
```
Consider leaving a comment as to how this value was derived (educated guess, average header length from source X, etc..)
sure why not
sure can do

Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackHuffmanDecoder.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackHuffmanDecoder.java
index 307d11a7d53..b49c30d69b5 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackHuffmanDecoder.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HpackHuffmanDecoder.java
@@ -231,8 +231,11 @@ AsciiString end() throws Http2Exception {
 
         private void append(int i) {
             if (bytes.length == index) {
-                // Always just expand by INITIAL_SIZE
-                byte[] newBytes = new byte[bytes.length + initialCapacity];
+                // Choose an expanding strategy depending on how big the buffer already is.
+                // 1024 was choosen as a good guess and we may be able to investigate more if there are better choices.
+                // See also https://github.com/netty/netty/issues/6846
+                final int newLength = bytes.length >= 1024 ? bytes.length + initialCapacity : bytes.length << 1;
+                byte[] newBytes = new byte[newLength];
                 System.arraycopy(bytes, 0, newBytes, 0, bytes.length);
                 bytes = newBytes;
             }


Test patch:
Repository: netty/netty
Issue: Typo? EpollSocketChannelConfig.setTcpKeepCntl

https://github.com/netty/netty/blob/3cc405296310643bccddc8c81998c97f25b3201c/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannelConfig.java#L420

shouldn't it be called setTcpKeepCnt?

Issue comments:
yep but it's public so it cannot be changed until the next major version.
It can not be changed but we can add a new method and deprecate the old one... let me do in a few minutes 
Fixed by https://github.com/netty/netty/pull/6910. Please check
Fixed by https://github.com/netty/netty/pull/6910

Pull request review comments:

Patch that resolved the issue:
diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannelConfig.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannelConfig.java
index 4ac335619b8..eaf2523b980 100644
--- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannelConfig.java
+++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannelConfig.java
@@ -141,7 +141,7 @@ public <T> boolean setOption(ChannelOption<T> option, T value) {
         } else if (option == EpollChannelOption.TCP_KEEPIDLE) {
             setTcpKeepIdle((Integer) value);
         } else if (option == EpollChannelOption.TCP_KEEPCNT) {
-            setTcpKeepCntl((Integer) value);
+            setTcpKeepCnt((Integer) value);
         } else if (option == EpollChannelOption.TCP_KEEPINTVL) {
             setTcpKeepIntvl((Integer) value);
         } else if (option == EpollChannelOption.TCP_USER_TIMEOUT) {
@@ -415,9 +415,17 @@ public EpollSocketChannelConfig setTcpKeepIntvl(int seconds) {
     }
 
     /**
-     * Set the {@code TCP_KEEPCNT} option on the socket. See {@code man 7 tcp} for more details.
+     * @deprecated use {@link #setTcpKeepCnt(int)}
      */
+    @Deprecated
     public EpollSocketChannelConfig setTcpKeepCntl(int probes) {
+        return setTcpKeepCnt(probes);
+    }
+
+    /**
+     * Set the {@code TCP_KEEPCNT} option on the socket. See {@code man 7 tcp} for more details.
+     */
+    public EpollSocketChannelConfig setTcpKeepCnt(int probes) {
         try {
             channel.socket.setTcpKeepCnt(probes);
             return this;


Test patch:
Repository: netty/netty
Issue: small Default max message size in MqttDecoder

### Expected behavior
According to [MQTT 3.1.1 spec, section 2.2.3](http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/csprd02/mqtt-v3.1.1-csprd02.html#_Toc385349759), MQTT packets size can take up to 268 435 455 bytes. I think this value should be a default. 

### Actual behavior
The current default value of MQTT packet to decode is 8092. 



Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java
index 8d94ee48e96..6789722a895 100644
--- a/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java
+++ b/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java
@@ -39,7 +39,7 @@
  */
 public final class MqttDecoder extends ReplayingDecoder<DecoderState> {
 
-    private static final int DEFAULT_MAX_BYTES_IN_MESSAGE = 8092;
+    private static final int DEFAULT_MAX_BYTES_IN_MESSAGE = 268435455;
 
     /**
      * States of the decoder.


Test patch:
Repository: netty/netty
Issue: DefaultSocks5CommandRequest incorrectly rejects SOCKS5 commands with dstPort=0
Labels: defect

### Expected behavior
According to [SOCKS 5 spec](https://www.ietf.org/rfc/rfc1928.txt), dstPort = 0 is a valid value in case of UDP ASSOCIATE command:

> If the client is not in possesion of the information at the time of the UDP ASSOCIATE, the client MUST use a port number and address of all zeros.

### Actual behavior
However, constructor of DefaultSocks5CommandRequest rejects this value as invalid:

    if (dstPort <= 0 || dstPort >= 65536) {
            throw new IllegalArgumentException("dstPort: " + dstPort + " (expected: 1~65535)");
    }

### Netty version
4.1.15.Final

Issue comments:
@axtavt please check https://github.com/netty/netty/pull/7157
Fixed by https://github.com/netty/netty/pull/7157

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequest.java b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequest.java
index d90150ebc24..ddd35fd6250 100755
--- a/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequest.java
+++ b/codec-socks/src/main/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequest.java
@@ -59,8 +59,8 @@ public DefaultSocks5CommandRequest(
             }
         }
 
-        if (dstPort <= 0 || dstPort >= 65536) {
-            throw new IllegalArgumentException("dstPort: " + dstPort + " (expected: 1~65535)");
+        if (dstPort < 0 || dstPort > 65535) {
+            throw new IllegalArgumentException("dstPort: " + dstPort + " (expected: 0~65535)");
         }
 
         this.type = type;


Test patch:
diff --git a/codec-socks/src/test/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequestTest.java b/codec-socks/src/test/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequestTest.java
index e4dc2dade7a..3b3b5db5281 100755
--- a/codec-socks/src/test/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequestTest.java
+++ b/codec-socks/src/test/java/io/netty/handler/codec/socksx/v5/DefaultSocks5CommandRequestTest.java
@@ -76,7 +76,7 @@ public void testIDNNotExceeds255CharsLimit() {
     public void testValidPortRange() {
         try {
             new DefaultSocks5CommandRequest(Socks5CommandType.BIND, Socks5AddressType.DOMAIN,
-                    "παράδειγμα.δοκιμήπαράδει", 0);
+                    "παράδειγμα.δοκιμήπαράδει", -1);
         } catch (Exception e) {
             assertTrue(e instanceof IllegalArgumentException);
         }
@@ -87,5 +87,10 @@ public void testValidPortRange() {
         } catch (Exception e) {
             assertTrue(e instanceof IllegalArgumentException);
         }
+
+        new DefaultSocks5CommandRequest(Socks5CommandType.BIND, Socks5AddressType.DOMAIN,
+                "παράδειγμα.δοκιμήπαράδει", 0);
+        new DefaultSocks5CommandRequest(Socks5CommandType.BIND, Socks5AddressType.DOMAIN,
+                "παράδειγμα.δοκιμήπαράδει", 65535);
     }
 }
Repository: netty/netty
Issue: netty-all artifact missing automatic-module-name
Labels: defect

The `netty-all` artifact doesn't have a `Automatic-Module-Name` defined in the manifest like the rest of the projects do, resulting in `requires netty.all`.

Issue comments:
Can you provide a pr for a fix ?

> Am 27.01.2018 um 10:08 schrieb kashike <notifications@github.com>:
> 
> The netty-all artifact doesn't have a Automatic-Module-Name defined in the manifest like the rest of the projects do, resulting in requires netty.all.
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

#7645

Pull request review comments:

Patch that resolved the issue:
diff --git a/README.md b/README.md
index d45b41a917f..5c4c4b5afd8 100644
--- a/README.md
+++ b/README.md
@@ -31,6 +31,7 @@ Netty can be used in modular JDK9 applications as a collection of automatic modu
 reverse-DNS style, and are derived from subproject names rather than root packages due to historical reasons. They
 are listed below:
 
+ * `io.netty.all`
  * `io.netty.buffer`
  * `io.netty.codec`
  * `io.netty.codec.dns`
diff --git a/all/pom.xml b/all/pom.xml
index b43869d6acc..f3e897d9a2b 100644
--- a/all/pom.xml
+++ b/all/pom.xml
@@ -664,6 +664,9 @@
                 <manifest>
                   <addDefaultImplementationEntries>true</addDefaultImplementationEntries>
                 </manifest>
+                <manifestEntries>
+                  <Automatic-Module-Name>io.netty.all</Automatic-Module-Name>
+                </manifestEntries>
                 <index>true</index>
               </archive>
             </configuration>


Test patch:
Repository: netty/netty
Issue: ResourceLeakDetector does not retain the initial record
Labels: defect

### Expected behavior

The leak report printed by `ResourceLeakDetector` retains the stack trace of the moment the buffer was created.

### Actual behavior

The record with `Created at:` heading is not actually the creation record, e.g.

```
Created at:
    io.netty.buffer.AdvancedLeakAwareByteBuf.writeBytes(AdvancedLeakAwareByteBuf.java:616)
    ...
```

### Steps to reproduce

Not sure, but probably write an app that generates a lot of access records and leak?

### Minimal yet complete reproducer code (or URL to code)

Sorry, no.

### Netty version

4.1.22

### JVM version (e.g. `java -version`)

```
java version "1.8.0_144"
Java(TM) SE Runtime Environment (build 1.8.0_144-b01)
Java HotSpot(TM) 64-Bit Server VM (build 25.144-b01, mixed mode)
```

### OS version (e.g. `uname -a`)

```
Linux infinity 4.14.25-1-lts #1 SMP Fri Mar 9 22:02:42 CET 2018 x86_64 GNU/Linux
```

Issue comments:
Will have a look
@trustin please check the PR.

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/ResourceLeakDetector.java b/common/src/main/java/io/netty/util/ResourceLeakDetector.java
index 3872c07d1cc..c29320adb7d 100644
--- a/common/src/main/java/io/netty/util/ResourceLeakDetector.java
+++ b/common/src/main/java/io/netty/util/ResourceLeakDetector.java
@@ -369,7 +369,8 @@ private static final class DefaultResourceLeak<T>
             // be collected via the WeakReference.
             trackedHash = System.identityHashCode(referent);
             allLeaks.put(this, LeakEntry.INSTANCE);
-            headUpdater.set(this, Record.BOTTOM);
+            // Create a new Record so we always have the creation stacktrace included.
+            headUpdater.set(this, new Record(Record.BOTTOM));
             this.allLeaks = allLeaks;
         }
 


Test patch:
Repository: netty/netty
Issue: promise already done caused by ChunkedWriteHandler writing the same (non chunked) message twice
Labels: defect

Hello,

I get an exception "promise already done" apparently caused by the ChunkedWriteHandler which is called twice for the same message, one from my flush and the other time from the channelWritabilityChanged.

Not sure if the ChunkedWriteHandler.doFlush method is supposed to be called twice in the same write but it looks to work ok when it's called twice and the message is ChunkedInput.

Maybe lines 304 and 305 should be reversed from:
```
                ctx.write(pendingMessage, currentWrite.promise);
                this.currentWrite = null;
```
to:
```
                this.currentWrite = null;
                ctx.write(pendingMessage, currentWrite.promise);
```

A stacktrace is attached.
[stack-trace.txt](https://github.com/netty/netty/files/1863541/stack-trace.txt)



Issue comments:
Yes this sounds good.... can you provide a PR?

> Am 30.03.2018 um 14:24 schrieb Nicolae Mihalache <notifications@github.com>:
> 
> Hello,
> 
> I get an exception "promise already done" apparently caused by the ChunkedWriteHandler which is called twice for the same message, one from my flush and the other time from the channelWritabilityChanged.
> 
> Not sure if the ChunkedWriteHandler.doFlush method is supposed to be called twice in the same write but it looks to work ok when it's called twice and the message is ChunkedInput.
> 
> Maybe lines 304 and 305 should be reversed from:
> 
>                 ctx.write(pendingMessage, currentWrite.promise);
>                 this.currentWrite = null;
> to:
> 
>                 this.currentWrite = null;
>                 ctx.write(pendingMessage, currentWrite.promise);
> A stacktrace is attached.
> stack-trace.txt
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 


Pull request review comments:

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java b/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java
index 1bc0c6bf81d..6f753c5c510 100644
--- a/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java
+++ b/handler/src/main/java/io/netty/handler/stream/ChunkedWriteHandler.java
@@ -299,8 +299,8 @@ public void operationComplete(ChannelFuture future) throws Exception {
                 ctx.flush();
                 requiresFlush = false;
             } else {
-                ctx.write(pendingMessage, currentWrite.promise);
                 this.currentWrite = null;
+                ctx.write(pendingMessage, currentWrite.promise);
                 requiresFlush = true;
             }
 


Test patch:
Repository: netty/netty
Issue: CorsHandler do writeAndFlush in override ChannelHandlerContext#write method

Do not really understand, why in `CorsHandler` invoked writeAndFlash ? It's strange side effect.
### Actual behavior

https://github.com/netty/netty/blob/473f6a6edb8353728ea8e711ac8241ddc21adf08/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java#L198-L209
### Steps to reproduce
Add `CorsHandler` to pipeline then write response in your own handler, response flushed automatically.

### Netty version
4.1.22


Issue comments:
@amizurov its a bug :(
 
Fixed https://github.com/netty/netty/pull/7839
@normanmaurer Thanks a lot, It's really quickly as well as Netty :)

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java
index f4b5552b724..9e7756521e9 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java
@@ -205,7 +205,7 @@ public void write(final ChannelHandlerContext ctx, final Object msg, final Chann
                 setExposeHeaders(response);
             }
         }
-        ctx.writeAndFlush(msg, promise);
+        ctx.write(msg, promise);
     }
 
     private static void forbidden(final ChannelHandlerContext ctx, final HttpRequest request) {


Test patch:
Repository: netty/netty
Issue: Eternal loop in Recycler.WeakOrderQueue.Head#run() that blocks ObjectCleaner thread and causes OOM.
Labels: defect

Hi,

looks like in commit 6eb9674bf5ded128cb230a31987f14816b7f1886 there is a problem that causes an eternal loop in `Recycler.WeakOrderQueue.Head#run()`:
```
            public void run() {
                Link head = link;
                while (head != null) {
                    reclaimSpace(LINK_CAPACITY);
                    head = link.next;
                }
            }
```
should be 
```
            public void run() {
                Link head = link;
                while (head != null) {
                    reclaimSpace(LINK_CAPACITY);
                    head = head.next;
                }
            }
```
Note `head = link.next` vs `head = head.next`

### Expected behavior

Recycler.WeakOrderQueue.Head#run() should not block forever.

### Actual behavior

Recycler.WeakOrderQueue.Head#run() falls into an eternal loop and blocks ObjectCleaner thread.

### Steps to reproduce

Please see below...

### Minimal yet complete reproducer code (or URL to code)

Sorry for not providing a reproducer, this issues came out in our project's test suite as an OOM after a long run and we were able to find its cause only by analyzing the JVM heap dump. 

### Netty version

4.1.23.Final

### JVM version (e.g. `java -version`)

java version "1.8.0_162"
Java(TM) SE Runtime Environment (build 1.8.0_162-b12)
Java HotSpot(TM) 64-Bit Server VM (build 25.162-b12, mixed mode)

### OS version (e.g. `uname -a`)

Darwin MacBook-Pro-sy.local 17.5.0 Darwin Kernel Version 17.5.0: Mon Mar  5 22:24:32 PST 2018; root:xnu-4570.51.1~1/RELEASE_X86_64 x86_64

Issue comments:
@syermakov OMG... looking. Thanks for reporting
Fixed by https://github.com/netty/netty/pull/7878

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/Recycler.java b/common/src/main/java/io/netty/util/Recycler.java
index fde88782218..e84adc5a9a6 100644
--- a/common/src/main/java/io/netty/util/Recycler.java
+++ b/common/src/main/java/io/netty/util/Recycler.java
@@ -261,7 +261,7 @@ public void run() {
                 Link head = link;
                 while (head != null) {
                     reclaimSpace(LINK_CAPACITY);
-                    head = link.next;
+                    head = head.next;
                 }
             }
 


Test patch:
Repository: netty/netty
Issue: Http2CodecUtil.java - emptyPingBuf() is no longer needed (as writePing takes a long)
Labels: cleanup

https://github.com/netty/netty/blob/b192bf12ad2af92bba0f32c9d3127b1192e54670/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2CodecUtil.java#L177


In https://github.com/netty/netty/commit/501662a77f8166382eefd18a8620983d27f59d4a - emptyPing data changed to long (instead of buffer data)

This should be updated.

Issue comments:
@vsabella I guess we should just remove it completely. 

@carl-mastrangelo @ejona86 @Scottmitch thoughts ?
Also @mosesn @bryce-anderson WDYT ?
Looks like dead code to me and `Http2CodecUtil` is still tagged `@UnstableApi` so removing it seems like the right move.
@bryce-anderson sounds good... Will try to find time to write a patch or if you want you could also if you have time 👍 
I can take it. Unless it is actually in use it should be trivial to remove.
Thanks that would be awesome

> Am 06.06.2018 um 14:28 schrieb Bryce Anderson <notifications@github.com>:
> 
> I can take it. Unless it is actually in use it should be trivial to remove.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.

Thanks @bryce-anderson 

Pull request review comments:

Patch that resolved the issue:
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 317ee48063d..7ebc8fd4888 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
@@ -67,9 +67,6 @@ public final class Http2CodecUtil {
     private static final ByteBuf CONNECTION_PREFACE =
             unreleasableBuffer(directBuffer(24).writeBytes("PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".getBytes(UTF_8)))
                     .asReadOnly();
-    private static final ByteBuf EMPTY_PING =
-            unreleasableBuffer(directBuffer(PING_FRAME_PAYLOAD_LENGTH).writeZero(PING_FRAME_PAYLOAD_LENGTH))
-                    .asReadOnly();
 
     private static final int MAX_PADDING_LENGTH_LENGTH = 1;
     public static final int DATA_FRAME_HEADER_LENGTH = FRAME_HEADER_LENGTH + MAX_PADDING_LENGTH_LENGTH;
@@ -169,14 +166,6 @@ public static ByteBuf connectionPrefaceBuf() {
         return CONNECTION_PREFACE.retainedDuplicate();
     }
 
-    /**
-     * Returns a buffer filled with all zeros that is the appropriate length for a PING frame.
-     */
-    public static ByteBuf emptyPingBuf() {
-        // Return a duplicate so that modifications to the reader index will not affect the original buffer.
-        return EMPTY_PING.retainedDuplicate();
-    }
-
     /**
      * Iteratively looks through the causality chain for the given exception and returns the first
      * {@link Http2Exception} or {@code null} if none.


Test patch: