diff --git a/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java b/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java index dfc740e35..e7aa7e790 100644 --- a/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java +++ b/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java @@ -15,6 +15,8 @@ import com.github.dockerjava.core.AbstractDockerCmdExecFactory; import com.github.dockerjava.core.WebTarget; +import io.netty.util.concurrent.Future; +import io.netty.util.concurrent.GenericFutureListener; import org.apache.commons.lang.SystemUtils; import com.github.dockerjava.api.command.DockerCmdExecFactory; @@ -232,12 +234,34 @@ public DuplexChannel connect(Bootstrap bootstrap) throws InterruptedException { throw new RuntimeException("no port configured for " + host); } - DuplexChannel channel = (DuplexChannel) bootstrap.connect(host, port).sync().channel(); + final DuplexChannel channel = (DuplexChannel) bootstrap.connect(host, port).sync().channel(); final SslHandler ssl = initSsl(dockerClientConfig); if (ssl != null) { channel.pipeline().addFirst(ssl); + + // https://tools.ietf.org/html/rfc5246#section-7.2.1 + // TLS has its own special message about connection termination. Because TLS is a + // session-level protocol, it can be covered by any transport-level protocol like + // TCP, UTP and so on. But we know exactly that data being transferred over TCP and + // that other side will never send any byte into this TCP connection, so this + // channel should be closed. + // RFC says that we must notify opposite side about closing. This could be done only + // in sun.security.ssl.SSLEngineImpl and unfortunately it does not send this + // message. On the other hand RFC does not enforce the opposite side to wait for + // such message. + ssl.sslCloseFuture().addListener(new GenericFutureListener>() { + @Override + public void operationComplete(Future future) { + channel.eventLoop().execute(new Runnable() { + @Override + public void run() { + channel.close(); + } + }); + } + }); } return channel; diff --git a/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java b/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java index bac45f63f..caaadde16 100644 --- a/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java +++ b/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java @@ -13,11 +13,14 @@ import org.slf4j.LoggerFactory; import java.io.ByteArrayInputStream; +import java.io.Closeable; import java.io.File; import java.io.InputStream; import java.io.PipedInputStream; import java.io.PipedOutputStream; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import static com.github.dockerjava.junit.DockerRule.DEFAULT_IMAGE; import static java.util.concurrent.TimeUnit.SECONDS; @@ -28,6 +31,7 @@ import static org.hamcrest.Matchers.isEmptyString; import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeThat; /** @@ -205,6 +209,68 @@ public void onNext(Frame frame) { callback.close(); } + /** + * {@link AttachContainerResultCallback#onComplete()} should be called immediately after + * container exit. It was broken for Netty and TLS connection. + */ + @Test + public void attachContainerClosesStdoutWhenContainerExits() throws Exception { + DockerClient dockerClient = dockerRule.getClient(); + + CreateContainerResponse container = dockerClient.createContainerCmd(DEFAULT_IMAGE) + .withCmd("echo", "hello world") + .withTty(false) + .exec(); + LOG.info("Created container: {}", container.toString()); + + final CountDownLatch started = new CountDownLatch(1); + final AtomicLong startedAtNanos = new AtomicLong(); + final CountDownLatch gotLine = new CountDownLatch(1); + final CountDownLatch completed = new CountDownLatch(1); + final AtomicLong gotLineAtNanos = new AtomicLong(); + AttachContainerTestCallback callback = new AttachContainerTestCallback() { + @Override + public void onStart(Closeable stream) { + startedAtNanos.set(System.nanoTime()); + started.countDown(); + super.onStart(stream); + } + + @Override + public void onNext(Frame item) { + if (item.getStreamType() == StreamType.STDOUT) { + gotLineAtNanos.set(System.nanoTime()); + gotLine.countDown(); + } + super.onNext(item); + } + + @Override + public void onComplete() { + completed.countDown(); + super.onComplete(); + } + }; + + try (Closeable ignored = callback) { + dockerClient.attachContainerCmd(container.getId()) + .withStdOut(true) + .withFollowStream(true) + .exec(callback); + + dockerClient.startContainerCmd(container.getId()).exec(); + + assertTrue("Should start in a reasonable time", started.await(30, SECONDS)); + assertTrue("Should get first line quickly after the start", gotLine.await(15, SECONDS)); + + long gotLineDurationSeconds = (gotLineAtNanos.get() - startedAtNanos.get()) / 1_000_000_000L; + LOG.info("Got line from {} for {} seconds", container.getId(), gotLineDurationSeconds); + + boolean finished = completed.await(1L + gotLineDurationSeconds, SECONDS); + assertTrue("Should get EOF in a time close to time of getting the first line", finished); + } + } + public static class AttachContainerTestCallback extends AttachContainerResultCallback { private StringBuffer log = new StringBuffer();