zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-06-27 23:03:34 +02:00
Code Issues Projects Releases Packages Wiki Activity

Make SSL failure test more robust

Browse source 
A recent change in Netty 4.1.21 (978a46cc0a) tried to fix an issue where channels might be closed before any handshake exception can be propagated. This however introduced a regression where the the connection is not closed at all after a handshake failure, which caused test failures because we were expecting the connection to be closed after a handshake failure.

We rolled back dependency on Netty 4.1.21 so that the test would pass. A fix upstream is schedule for 4.1.22 (https://github.com/netty/netty/pull/7727).

However this does reveal some potential problem in our tests. Namely we did not wait for the connection to be closed before assertion on it. The old Netty behavior closes the connection before handshake exception is thrown, and we *do* wait for the handshake exception. The connection assertion happens after the handshake exception is verified, so by then the connection is always closed.

When the upstream fix is released, we'd run into concurrency problem described above. So we instead wait for the connection to be closed before checking handshake exception (by releasing the lock in a channel close listener), which guarantees that when we check the connection, it is always closed.

Also fixes some javadoc errors.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=186021997
This commit is contained in:
jianglai 2018-02-16 11:23:09 -08:00
parent 7f2e3695a6
commit 1013e047b4
1 changed files with 10 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

20
javatests/google/registry/proxy/handler/SslInitializerTestUtils.java
View file

@ -73,7 +73,7 @@ public class SslInitializerTestUtils {
// Only use one thread in the event loop group. The same event loop group will be used to // Only use one thread in the event loop group. The same event loop group will be used to
// register client channels during setUpClient as well. This ensures that all I/O activities // register client channels during setUpClient as well. This ensures that all I/O activities
// in both channels happen in the same thread, making debugging easier (i. e. no need to jump // in both channels happen in the same thread, making debugging easier (i. e. no need to jump
// between threads when debugging, everything happens synchronously within the only I/O // between threads when debugging, everything happens synchronously within the only I/O thread
// effectively). Note that the main thread is still separate from the I/O thread and // effectively). Note that the main thread is still separate from the I/O thread and
// synchronization (using the lock field) is still needed when the main thread needs to verify // synchronization (using the lock field) is still needed when the main thread needs to verify
// properties calculated by the I/O thread. // properties calculated by the I/O thread.
@ -143,9 +143,9 @@ public class SslInitializerTestUtils {
@Override @Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
serverException.initCause(cause); serverException.initCause(cause);
// If an exception is caught, we should also release the lock so that the main thread knows // If an exception is caught, we should also release the lock after the channel is closed
// there is an exception to inspect now. // so that the main thread knows there is an exception to inspect now.
lock.unlock(); ctx.channel().closeFuture().addListener(f -> lock.unlock());
} }
} }
@ -153,9 +153,9 @@ public class SslInitializerTestUtils {
static class DumpHandler extends ChannelInboundHandlerAdapter { static class DumpHandler extends ChannelInboundHandlerAdapter {
/** /**
* A lock that synchronizes server I/O activity with the main thread. Acquired by the server I/O * A lock that synchronizes client I/O activity with the main thread. Acquired by the client I/O
* thread when the handler is constructed, released when the server echoes back, or when an * thread when the handler is constructed, released when the client receives an response, or
* exception is caught (during SSH handshake for example). * when an exception is caught (during SSH handshake for example).
*/ */
private final Lock lock; private final Lock lock;
@ -197,9 +197,9 @@ public class SslInitializerTestUtils {
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
clientException.initCause(cause); clientException.initCause(cause);
// If an exception is caught here, the main thread must be waiting to acquire the lock from // If an exception is caught here, the main thread must be waiting to acquire the lock from
// the I/O thread in order to verify it. Releasing the lock to notify the main thread it can // the I/O thread in order to verify it. Releasing the lock after the channel is closed to
// continue now that the exception has been written. // notify the main thread it can continue now that the exception has been written.
lock.unlock(); ctx.channel().closeFuture().addListener(f -> lock.unlock());
} }
} }