feedback-to-code/mozzarella huggingface dataset:
Repository: netty/netty
Issue: Race in AddressResolverGroup
Labels: defect

https://github.com/netty/netty/blob/4.1/resolver/src/main/java/io/netty/resolver/AddressResolverGroup.java

Here, `resolvers` is synchronized when calling .get(), but not in the future callback `resolvers.remove(executor);`


Issue comments:
cc: @nmittler & @normanmaurer

Why not use a CHMv8 here? It seems the original concern was `AddressResolver` instantiation cost, but that's what `computeIfAbsent` is for. This would let us remove all the synchronized blocks.

Because it's java8 only... Will fix the race though

> Am 03.02.2016 um 23:09 schrieb Stephane Landelle notifications@github.com:
> 
> Why not use a CHMv8 here? It seems the original concern was AddressResolver instantiation cost, but that's what computeIfAbsent is for. This would let us remove all the synchronized blocks.
> 
> —
> Reply to this email directly or view it on GitHub.

@normanmaurer I'm referring to your internal fork: https://github.com/netty/netty/blob/4.1/common/src/main/java/io/netty/util/internal/chmv8/ConcurrentHashMapV8.java

@trustin WDYT?

> Am 04.02.2016 um 08:08 schrieb Stephane Landelle notifications@github.com:
> 
> @normanmaurer I'm referring to your internal fork: https://github.com/netty/netty/blob/4.1/common/src/main/java/io/netty/util/internal/chmv8/ConcurrentHashMapV8.java
> 
> —
> Reply to this email directly or view it on GitHub.

Fixed by https://github.com/netty/netty/pull/4844


Pull request review comments:

Patch that resolved the issue:
diff --git a/resolver/src/main/java/io/netty/resolver/AddressResolverGroup.java b/resolver/src/main/java/io/netty/resolver/AddressResolverGroup.java
index 89b00dfa044..509475eb3ea 100644
--- a/resolver/src/main/java/io/netty/resolver/AddressResolverGroup.java
+++ b/resolver/src/main/java/io/netty/resolver/AddressResolverGroup.java
@@ -73,7 +73,9 @@ public AddressResolver<T> getResolver(final EventExecutor executor) {
                 executor.terminationFuture().addListener(new FutureListener<Object>() {
                     @Override
                     public void operationComplete(Future<Object> future) throws Exception {
-                        resolvers.remove(executor);
+                        synchronized (resolvers) {
+                            resolvers.remove(executor);
+                        }
                         newResolver.close();
                     }
                 });


Test patch:
Repository: netty/netty
Issue: CorsConfigBuilder allowNullOrigin method should be public instead of package private
Labels: defect

Netty version: 4.1.0-CR1

Class:
io.netty.handler.codec.http.cors.CorsConfigBuilder

Method:
method "allowNullOrigin" should be public but it is package private (no modifier)


Issue comments:
sgtm

Fixed in #4852

Thanks for the patch @windie !


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java
index 81e0a8a2137..52457b32317 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsConfigBuilder.java
@@ -103,7 +103,7 @@ public static CorsConfigBuilder forOrigins(final String... origins) {
      *
      * @return {@link CorsConfigBuilder} to support method chaining.
      */
-    CorsConfigBuilder allowNullOrigin() {
+    public CorsConfigBuilder allowNullOrigin() {
         allowNullOrigin = true;
         return this;
     }


Test patch:
Repository: netty/netty
Issue: Build warning message due to missing  paxexam version
Labels: cleanup

Netty version: 4.1.0.CR4-SNAPSHOT

Context:
I have noticed below warning message[1] when building netty project. 

[1].
[WARNING] Some problems were encountered while building the effective model for io.netty:netty-testsuite-osgi:jar:4.1.0.CR4-SNAPSHOT
[WARNING] 'build.plugins.plugin.version' for org.ops4j.pax.exam:maven-paxexam-plugin is missing. @ io.netty:netty-testsuite-osgi:[unknown-version], /home/chandana/Documents/branches/public/git/netty/testsuite-osgi/pom.xml, line 220, column 15
[WARNING] 

Steps to reproduce:
1. Try to build netty project
2. warning message will be logged in the stacktrace.

$ java -version
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)

Operating system: Ubuntu Linux 13.04 64-bit

$ uname -a
Linux chandana-Latitude-E6540 3.13.0-79-generic #123-Ubuntu SMP Fri Feb 19 14:27:58 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux


Issue comments:
Sent a PR[1] to fix this.

[1]. https://github.com/netty/netty/pull/4910

Fixed by https://github.com/netty/netty/pull/4910


Pull request review comments:

Patch that resolved the issue:
diff --git a/pom.xml b/pom.xml
index b38fb3b30f3..dd5aaf4a6bd 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1009,6 +1009,11 @@
           <artifactId>maven-resources-plugin</artifactId>
           <version>2.6</version>
         </plugin>
+        <plugin>
+          <groupId>org.ops4j.pax.exam</groupId>
+          <artifactId>maven-paxexam-plugin</artifactId>
+          <version>1.2.4</version>
+        </plugin>
         <plugin>
           <artifactId>maven-jar-plugin</artifactId>
           <version>2.5</version>


Test patch:
Repository: netty/netty
Issue: Print the full thread dump on test timeout
Labels: cleanup

Otherwise it is sometimes difficult to analyze the test timeout reported by our CI server - e.g. #3138 

We could write a JUnit `RunListener` that prints the full thread dump when the test fails with timeout, and also do some additional operations like triggering GC when running a leak test.

The Maven surefire plugin allows us to specify the listener in the pom:

```
          <properties>
            <property>
              <name>listener</name>
              <value>...NettyRunListener</value>
            </property>
          </properties>
```


Issue comments:
Fixed by https://github.com/netty/netty/pull/4931


Pull request review comments:

Patch that resolved the issue:
diff --git a/pom.xml b/pom.xml
index 0697eeb437a..8264f7f8d44 100644
--- a/pom.xml
+++ b/pom.xml
@@ -201,6 +201,7 @@
   <properties>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
+    <netty.build.version>22</netty.build.version>
     <jboss.marshalling.version>1.3.18.GA</jboss.marshalling.version>
     <jetty.alpnAgent.version>1.0.1.Final</jetty.alpnAgent.version>
     <jetty.alpnAgent.path>${settings.localRepository}/kr/motd/javaagent/jetty-alpn-agent/${jetty.alpnAgent.version}/jetty-alpn-agent-${jetty.alpnAgent.version}.jar</jetty.alpnAgent.path>
@@ -420,6 +421,12 @@
         <version>4.12</version>
         <scope>test</scope>
       </dependency>
+      <dependency>
+        <groupId>${project.groupId}</groupId>
+        <artifactId>netty-build</artifactId>
+        <version>${netty.build.version}</version>
+        <scope>test</scope>
+      </dependency>
       <dependency>
         <groupId>org.hamcrest</groupId>
         <artifactId>hamcrest-library</artifactId>
@@ -532,6 +539,11 @@
       <artifactId>junit</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>netty-build</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest-library</artifactId>
@@ -714,7 +726,7 @@
           <dependency>
             <groupId>${project.groupId}</groupId>
             <artifactId>netty-build</artifactId>
-            <version>21</version>
+            <version>${netty.build.version}</version>
           </dependency>
         </dependencies>
       </plugin>
@@ -774,6 +786,12 @@
           </excludes>
           <runOrder>random</runOrder>
           <argLine>${argLine.common} ${argLine.alpnAgent} ${argLine.leak} ${argLine.coverage} ${argLine.noUnsafe}</argLine>
+          <properties>
+            <property>
+              <name>listener</name>
+              <value>io.netty.build.junit.TimedOutTestsListener</value>
+            </property>
+          </properties>
         </configuration>
       </plugin>
       <!-- always produce osgi bundles -->


Test patch:
Repository: netty/netty
Issue: Minor typo in DefaultStompFrame.toString()
Labels: cleanup

Hi,

in current Netty 4.1.x, the DefaultStompFrame.toString() method writes a different class name (Default**Full**StompFrame)  than it actually is... I think it's a minor typo, however for new users of the Stomp codec it's confusing when you cannot locate the class at first.

https://github.com/netty/netty/blob/4.1/codec-stomp/src/main/java/io/netty/handler/codec/stomp/DefaultStompFrame.java#L97

Best regards,
Stefan


Issue comments:
Thanks for reporting. I will take care.


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-stomp/src/main/java/io/netty/handler/codec/stomp/DefaultStompFrame.java b/codec-stomp/src/main/java/io/netty/handler/codec/stomp/DefaultStompFrame.java
index f4009608a32..5295cb64157 100644
--- a/codec-stomp/src/main/java/io/netty/handler/codec/stomp/DefaultStompFrame.java
+++ b/codec-stomp/src/main/java/io/netty/handler/codec/stomp/DefaultStompFrame.java
@@ -94,7 +94,7 @@ public boolean release(int decrement) {
 
     @Override
     public String toString() {
-        return "DefaultFullStompFrame{" +
+        return "DefaultStompFrame{" +
             "command=" + command +
             ", headers=" + headers +
             ", content=" + content.toString(CharsetUtil.UTF_8) +


Test patch:
Repository: netty/netty
Issue: EpollChannelOption.TCP_QUICKACK is set as an Integer
Labels: defect

Should be a Boolean.

Reported here:
https://github.com/netty/netty-tcnative/issues/128


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollChannelOption.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollChannelOption.java
index 2598d53a1c0..261a6f7796e 100644
--- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollChannelOption.java
+++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollChannelOption.java
@@ -35,7 +35,7 @@ public final class EpollChannelOption<T> extends ChannelOption<T> {
     public static final ChannelOption<Boolean> IP_FREEBIND = ChannelOption.valueOf("IP_FREEBIND");
     public static final ChannelOption<Integer> TCP_FASTOPEN = valueOf(T, "TCP_FASTOPEN");
     public static final ChannelOption<Integer> TCP_DEFER_ACCEPT = ChannelOption.valueOf(T, "TCP_DEFER_ACCEPT");
-    public static final ChannelOption<Integer> TCP_QUICKACK = ChannelOption.valueOf(T, "TCP_QUICKACK");
+    public static final ChannelOption<Boolean> TCP_QUICKACK = ChannelOption.valueOf(T, "TCP_QUICKACK");
 
     public static final ChannelOption<DomainSocketReadMode> DOMAIN_SOCKET_READ_MODE =
             ChannelOption.valueOf(T, "DOMAIN_SOCKET_READ_MODE");


Test patch:
Repository: netty/netty
Issue: EpollEventLoopGroup missing Executor-based constructor
Labels: feature

EpollEventLoop is sold as a drop-in replacement for NioEventLoop (at least on Linux), but EpollEventLoopGroup only has the ThreadFactory-based constructor, not the Executor-based constructor that NioEventLoopGroup also includes.


Issue comments:
thanks for reporting @alugowski 


Pull request review comments:

Patch that resolved the issue:
diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java
index c151806c66b..6a7b3162bbf 100644
--- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java
+++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoopGroup.java
@@ -15,8 +15,8 @@
  */
 package io.netty.channel.epoll;
 
-import io.netty.channel.EventLoop;
 import io.netty.channel.DefaultSelectStrategyFactory;
+import io.netty.channel.EventLoop;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.MultithreadEventLoopGroup;
 import io.netty.channel.SelectStrategyFactory;
@@ -50,7 +50,7 @@ public EpollEventLoopGroup(int nThreads) {
      */
     @SuppressWarnings("deprecation")
     public EpollEventLoopGroup(int nThreads, SelectStrategyFactory selectStrategyFactory) {
-        this(nThreads, null, selectStrategyFactory);
+        this(nThreads, (ThreadFactory) null, selectStrategyFactory);
     }
 
     /**
@@ -61,6 +61,10 @@ public EpollEventLoopGroup(int nThreads, ThreadFactory threadFactory) {
         this(nThreads, threadFactory, 0);
     }
 
+    public EpollEventLoopGroup(int nThreads, Executor executor) {
+        this(nThreads, executor, DefaultSelectStrategyFactory.INSTANCE);
+    }
+
     /**
      * Create a new instance using the specified number of threads and the given {@link ThreadFactory}.
      */
@@ -93,6 +97,10 @@ public EpollEventLoopGroup(int nThreads, ThreadFactory threadFactory, int maxEve
         super(nThreads, threadFactory, maxEventsAtOnce, selectStrategyFactory);
     }
 
+    public EpollEventLoopGroup(int nThreads, Executor executor, SelectStrategyFactory selectStrategyFactory) {
+        super(nThreads, executor, 0, selectStrategyFactory);
+    }
+
     /**
      * Sets the percentage of the desired amount of time spent for I/O in the child event loops.  The default value is
      * {@code 50}, which means the event loop will try to spend the same amount of time for I/O as for non-I/O tasks.


Test patch:
Repository: netty/netty
Issue: wrong huge deallocation counting in PoolArena.free
Labels: defect

right now there's `deallocationsHuge.decrement();`
should be `deallocationsHuge.increment();`


Issue comments:
@nayato you are right... let me fix it.

Fixed https://github.com/netty/netty/pull/5219


Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/PoolArena.java b/buffer/src/main/java/io/netty/buffer/PoolArena.java
index 1df94e493c3..e667d936c05 100644
--- a/buffer/src/main/java/io/netty/buffer/PoolArena.java
+++ b/buffer/src/main/java/io/netty/buffer/PoolArena.java
@@ -254,7 +254,7 @@ void free(PoolChunk<T> chunk, long handle, int normCapacity, PoolThreadCache cac
             int size = chunk.chunkSize();
             destroyChunk(chunk);
             activeBytesHuge.add(-size);
-            deallocationsHuge.decrement();
+            deallocationsHuge.increment();
         } else {
             SizeClass sizeClass = sizeClass(normCapacity);
             if (cache != null && cache.add(this, chunk, handle, normCapacity, sizeClass)) {


Test patch:
Repository: netty/netty
Issue: question on DefaultChannelPipeline.executorSafe - why indirection?
Labels: cleanup

Is there a particular reason to access channel-assigned executor like so `channel.unsafe().invoker().executor()` vs `channel.eventLoop()`?


Issue comments:
@nayato there is no good reason... let me fix it.

Fixed by https://github.com/netty/netty/pull/5251


Pull request review comments:

Patch that resolved the issue:
diff --git a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
index 5e56a974069..85696240a55 100644
--- a/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
+++ b/transport/src/main/java/io/netty/channel/DefaultChannelPipeline.java
@@ -1286,7 +1286,7 @@ private EventExecutor executorSafe(ChannelHandlerInvoker invoker) {
             // We check for channel().isRegistered and handlerAdded because even if isRegistered() is false we
             // can safely access the invoker() if handlerAdded is true. This is because in this case the Channel
             // was previously registered and so we can still access the old EventLoop to dispatch things.
-            return channel.isRegistered() || registered ? channel.unsafe().invoker().executor() : null;
+            return channel.isRegistered() || registered ? channel.eventLoop() : null;
         }
         return invoker.executor();
     }


Test patch:
Repository: netty/netty
Issue: OpensslEngine throws "session id context uninitialized" when used with client authentication
Labels: defect

I use netty to implement a webserver. With JDK ssl provider all work well but with open ssl provider i see the following errors when connection is made from a firefox browser and client authentication (mutual ssl) is requested. Without client authentication the error does not occur.

<pre>
javax.net.ssl.SSLHandshakeException: error:140D9115:SSL routines:ssl_get_prev_session:session id context uninitialized
    at io.netty.handler.ssl.OpenSslEngine.shutdownWithError(OpenSslEngine.java:575)
    at io.netty.handler.ssl.OpenSslEngine.sslReadErrorResult(OpenSslEngine.java:778)
    at io.netty.handler.ssl.OpenSslEngine.unwrap(OpenSslEngine.java:733)
    at io.netty.handler.ssl.OpenSslEngine.unwrap(OpenSslEngine.java:810)
</pre>


When used with chrome i see sometimes also this error

<pre>
error:140A1175:SSL routines:ssl_bytes_to_cipher_list:inappropriate fallback
</pre>


When used with safari i see sometimes also this error

<pre>
error:140760FC:SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol
</pre>


After a little bit of research this issue look like:
http://openssl.6102.n7.nabble.com/error-140D9115-SSL-routines-SSL-GET-PREV-SESSION-session-id-context-uninitialized-td5749.html
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=729480
https://jira.mongodb.org/browse/SERVER-9307

Netty: 4.0.36 with Fork15 of netty tcnative and OpenSSL 1.0.2e 3 Dec 2015


Issue comments:
PR https://github.com/netty/netty/pull/5321

Fixed by https://github.com/netty/netty/pull/5321


Pull request review comments:
Please define a static byte[] and reuse everytime. Something like:

private static final byte[] ID = new byte[] {'n', 'e', 't', 't', 'y'};


Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
index 156843eb63c..f069eaa4927 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
@@ -36,6 +36,7 @@
  * A server-side {@link SslContext} which uses OpenSSL's SSL/TLS implementation.
  */
 public final class OpenSslServerContext extends OpenSslContext {
+    private static final byte[] ID = new byte[] {'n', 'e', 't', 't', 'y'};
     private final OpenSslServerSessionContext sessionContext;
 
     /**
@@ -439,6 +440,7 @@ void verify(OpenSslEngine engine, X509Certificate[] peerCerts, String auth)
                 }
             }
             sessionContext = new OpenSslServerSessionContext(ctx);
+            sessionContext.setSessionIdContext(ID);
             success = true;
         } finally {
             if (!success) {


Test patch:
Repository: netty/netty
Issue: OpenSslServerContext NullPointEexception when using a KeyManagerFactory
Labels: defect

Support for using a KeyManagerFactory with OpenSSL was added by [PR 5493](https://github.com/netty/netty/pull/5439). However, there are a couple lingering `checkNotNull()` [assertions](https://github.com/netty/netty/blob/4.1/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java#L352) that cause a NullPointerException when using a KeyManagerFactory instead of a certificate-chain/private-key pair.

I think it's intended that these variables can be null now since there's an `if (keyCertChain != null)` check on line 370.


Issue comments:
I added a PR for this: #5567 

Fixed by https://github.com/netty/netty/pull/5567


Pull request review comments:

Patch that resolved the issue:
diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
index 9db007a7d6b..4986388d07a 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java
@@ -349,9 +349,6 @@ private OpenSslServerContext(
         // Create a new SSL_CTX and configure it.
         boolean success = false;
         try {
-            checkNotNull(keyCertChain, "keyCertChainFile");
-            checkNotNull(key, "keyFile");
-
             synchronized (OpenSslContext.class) {
                 try {
                     SSLContext.setVerify(ctx, SSL.SSL_CVERIFY_NONE, VERIFY_DEPTH);
@@ -373,9 +370,7 @@ private OpenSslServerContext(
                         }
 
                         if (keyManagerFactory != null) {
-                            X509KeyManager keyManager = chooseX509KeyManager(
-                                    buildKeyManagerFactory(keyCertChain, key, keyPassword, keyManagerFactory)
-                                            .getKeyManagers());
+                            X509KeyManager keyManager = chooseX509KeyManager(keyManagerFactory.getKeyManagers());
                             keyMaterialManager = useExtendedKeyManager(keyManager) ?
                                     new OpenSslExtendedKeyMaterialManager(
                                             (X509ExtendedKeyManager) keyManager, keyPassword) :


Test patch:
Repository: netty/netty
Issue: Proposal : remove commons logging classes
Labels: cleanup

Remove `CommonsLogger` and `CommonsLoggerFactory` classes.

Commons-logging project is not updated for 2 years already. I propose to remove it also from netty too.

Let me know If I can submit PR.


Issue comments:
I agree, I prefer `SLF4J-API` and `Logback`

it is causing any harm? maybe we can just mark it as deprecated, comment it will be removed in future releases, and recommend using an alternative (e.g. slf4j).

> it is causing any harm?

No harm. It is very outdated and not used within netty. Deprecation also fine.

+1 for deprecated.

Fixed


Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/internal/logging/CommonsLogger.java b/common/src/main/java/io/netty/util/internal/logging/CommonsLogger.java
index 1068005e893..110f0f1cf92 100644
--- a/common/src/main/java/io/netty/util/internal/logging/CommonsLogger.java
+++ b/common/src/main/java/io/netty/util/internal/logging/CommonsLogger.java
@@ -44,7 +44,11 @@
 /**
  * <a href="http://commons.apache.org/logging/">Apache Commons Logging</a>
  * logger.
+ *
+ * @deprecated Please use {@link Log4J2Logger} or {@link Log4JLogger} or
+ * {@link Slf4JLogger}.
  */
+@Deprecated
 class CommonsLogger extends AbstractInternalLogger {
 
     private static final long serialVersionUID = 8647838678388394885L;
diff --git a/common/src/main/java/io/netty/util/internal/logging/CommonsLoggerFactory.java b/common/src/main/java/io/netty/util/internal/logging/CommonsLoggerFactory.java
index 3e9ab9fafdb..2b4af9f4e9b 100644
--- a/common/src/main/java/io/netty/util/internal/logging/CommonsLoggerFactory.java
+++ b/common/src/main/java/io/netty/util/internal/logging/CommonsLoggerFactory.java
@@ -18,14 +18,15 @@
 
 import org.apache.commons.logging.LogFactory;
 
-import java.util.HashMap;
-import java.util.Map;
-
 /**
  * Logger factory which creates an
  * <a href="http://commons.apache.org/logging/">Apache Commons Logging</a>
  * logger.
+ *
+ * @deprecated Please use {@link Log4J2LoggerFactory} or {@link Log4JLoggerFactory} or
+ * {@link Slf4JLoggerFactory}.
  */
+@Deprecated
 public class CommonsLoggerFactory extends InternalLoggerFactory {
 
     public static final InternalLoggerFactory INSTANCE = new CommonsLoggerFactory();


Test patch: