andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix race in SslHandlerTest that could lead to NPE. (#7989)

Browse Source 
Motivation:

SslHandlerTest tried to get access to the SslHandler in the pipeline via pipeline.get(...) which may return null if the channel was already closed and so the pipeline was teared down.

This showed up in a test run as:

```
-------------------------------------------------------------------------------
Test set: io.netty.handler.ssl.SslHandlerTest
-------------------------------------------------------------------------------
Tests run: 17, Failures: 0, Errors: 1, Skipped: 1, Time elapsed: 0.802 sec <<< FAILURE! - in io.netty.handler.ssl.SslHandlerTest
testCloseOnHandshakeFailure(io.netty.handler.ssl.SslHandlerTest)  Time elapsed: 0.188 sec  <<< ERROR!
java.lang.NullPointerException
        at io.netty.handler.ssl.SslHandlerTest.testCloseOnHandshakeFailure(SslHandlerTest.java:640)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:564)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
        at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.lang.Thread.run(Thread.java:844)
```

Modifications:

Use an AtomicReference to propagate the SslHandler instance to the outer scope.

Result:

No more NPE.
This commit is contained in:
Norman Maurer 2018-05-30 22:07:42 +02:00 committed by GitHub
parent 48911e0b63
commit a4393831f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 10 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
View File

@ -64,6 +64,7 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
@ -626,17 +627,24 @@ public class SslHandlerTest {
});
sc = sb.bind(address).syncUninterruptibly().channel();
final AtomicReference<SslHandler> sslHandlerRef = new AtomicReference<SslHandler>();
Bootstrap b = new Bootstrap()
.group(group)
.channel(LocalChannel.class)
.handler(new ChannelInitializer<Channel>() {
@Override
protected void initChannel(Channel ch) {
ch.pipeline().addLast(sslClientCtx.newHandler(ch.alloc()));
SslHandler handler = sslClientCtx.newHandler(ch.alloc());
// We propagate the SslHandler via an AtomicReference to the outer-scope as using
// pipeline.get(...) may return null if the pipeline was teared down by the time we call it.
// This will happen if the channel was closed in the meantime.
sslHandlerRef.set(handler);
ch.pipeline().addLast(handler);
}
});
cc = b.connect(sc.localAddress()).syncUninterruptibly().channel();
SslHandler handler = cc.pipeline().get(SslHandler.class);
SslHandler handler = sslHandlerRef.get();
handler.handshakeFuture().awaitUninterruptibly();
assertFalse(handler.handshakeFuture().isSuccess());