From aca9a935b85710824bd1e293da1dc64cd4a881c0 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Wed, 18 Apr 2018 07:18:05 +0200 Subject: [PATCH] #369 Daemonize threads (#646) * #369 Mark all long-living threads started by Testcontainers as daemons and group them * simplify the classpath logic * fixes * Add a line about the logs to CHANGELOG and explain the DaemonTest and why we fork --- CHANGELOG.md | 2 + core/build.gradle | 4 +- .../testcontainers/DockerClientFactory.java | 1 + .../containers/GenericContainer.java | 4 +- .../DockerClientProviderStrategy.java | 4 +- .../TestcontainersDockerCmdExecFactory.java | 304 ++++++++++++++++++ .../images/builder/ImageFromDockerfile.java | 2 +- .../testcontainers/utility/MountableFile.java | 3 +- .../utility/ResourceReaper.java | 3 +- .../java/org/testcontainers/DaemonTest.java | 61 ++++ 10 files changed, 379 insertions(+), 9 deletions(-) create mode 100644 core/src/main/java/org/testcontainers/dockerclient/transport/TestcontainersDockerCmdExecFactory.java create mode 100644 core/src/test/java/org/testcontainers/DaemonTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d43b67e7b5..932893f84b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable changes to this project will be documented in this file. ### Changed - Support multiple HTTP status codes for HttpWaitStrategy ([\#630](https://github.com/testcontainers/testcontainers-java/issues/630)) +- Mark all long-living threads started by Testcontainers as daemons and group them. ([\#646](https://github.com/testcontainers/testcontainers-java/issues/646)) +- Remove noisy `DEBUG` logging of Netty packets ([\#646](https://github.com/testcontainers/testcontainers-java/issues/646)) ## [1.7.0] - 2018-04-07 diff --git a/core/build.gradle b/core/build.gradle index 645e2581b86..bf7bda5a8bf 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -106,10 +106,10 @@ dependencies { compile 'org.apache.commons:commons-compress:1.15' // Added for JDK9 compatibility since it's missing this package compile 'javax.xml.bind:jaxb-api:2.3.0' - compile 'org.rnorth.duct-tape:duct-tape:1.0.6' + compile 'org.rnorth.duct-tape:duct-tape:1.0.7' compile 'org.rnorth.visible-assertions:visible-assertions:2.1.0' - shaded ('com.github.docker-java:docker-java:3.0.12') { + shaded ('com.github.docker-java:docker-java:3.1.0-rc-2') { exclude(group: 'org.glassfish.jersey.core') exclude(group: 'org.glassfish.jersey.connectors') exclude(group: 'log4j') diff --git a/core/src/main/java/org/testcontainers/DockerClientFactory.java b/core/src/main/java/org/testcontainers/DockerClientFactory.java index 22f31123f5a..499c4ee4d85 100644 --- a/core/src/main/java/org/testcontainers/DockerClientFactory.java +++ b/core/src/main/java/org/testcontainers/DockerClientFactory.java @@ -39,6 +39,7 @@ @Slf4j public class DockerClientFactory { + public static final ThreadGroup TESTCONTAINERS_THREAD_GROUP = new ThreadGroup("testcontainers"); public static final String TESTCONTAINERS_LABEL = DockerClientFactory.class.getPackage().getName(); public static final String TESTCONTAINERS_SESSION_ID_LABEL = TESTCONTAINERS_LABEL + ".sessionId"; diff --git a/core/src/main/java/org/testcontainers/containers/GenericContainer.java b/core/src/main/java/org/testcontainers/containers/GenericContainer.java index 7d3001a5651..527e391718d 100644 --- a/core/src/main/java/org/testcontainers/containers/GenericContainer.java +++ b/core/src/main/java/org/testcontainers/containers/GenericContainer.java @@ -321,7 +321,7 @@ protected Path createVolumeDirectory(boolean temporary) { Path directory = new File(".tmp-volume-" + System.currentTimeMillis()).toPath(); PathUtils.mkdirp(directory); - if (temporary) Runtime.getRuntime().addShutdownHook(new Thread(() -> { + if (temporary) Runtime.getRuntime().addShutdownHook(new Thread(DockerClientFactory.TESTCONTAINERS_THREAD_GROUP, () -> { PathUtils.recursiveDeleteDir(directory); })); @@ -478,7 +478,7 @@ private void applyConfiguration(CreateContainerCmd createCommand) { private Set findLinksFromThisContainer(String alias, LinkableContainer linkableContainer) { return dockerClient.listContainersCmd() - .withStatusFilter("running") + .withStatusFilter(Arrays.asList("running")) .exec().stream() .flatMap(container -> Stream.of(container.getNames())) .filter(name -> name.endsWith(linkableContainer.getContainerName())) diff --git a/core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java b/core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java index f4dc5b2c4dd..f078aeac5fc 100644 --- a/core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java +++ b/core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java @@ -3,7 +3,6 @@ import com.github.dockerjava.api.DockerClient; import com.github.dockerjava.core.DockerClientBuilder; import com.github.dockerjava.core.DockerClientConfig; -import com.github.dockerjava.netty.NettyDockerCmdExecFactory; import com.google.common.base.Throwables; import org.apache.commons.io.IOUtils; import org.jetbrains.annotations.Nullable; @@ -13,6 +12,7 @@ import org.rnorth.ducttape.unreliables.Unreliables; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.testcontainers.dockerclient.transport.TestcontainersDockerCmdExecFactory; import org.testcontainers.utility.TestcontainersConfiguration; import java.util.ArrayList; @@ -166,7 +166,7 @@ public DockerClient getClient() { protected DockerClient getClientForConfig(DockerClientConfig config) { return DockerClientBuilder .getInstance(config) - .withDockerCmdExecFactory(new NettyDockerCmdExecFactory()) + .withDockerCmdExecFactory(new TestcontainersDockerCmdExecFactory()) .build(); } diff --git a/core/src/main/java/org/testcontainers/dockerclient/transport/TestcontainersDockerCmdExecFactory.java b/core/src/main/java/org/testcontainers/dockerclient/transport/TestcontainersDockerCmdExecFactory.java new file mode 100644 index 00000000000..1a0c52df5ac --- /dev/null +++ b/core/src/main/java/org/testcontainers/dockerclient/transport/TestcontainersDockerCmdExecFactory.java @@ -0,0 +1,304 @@ +package org.testcontainers.dockerclient.transport; + +import com.github.dockerjava.api.command.DockerCmdExecFactory; +import com.github.dockerjava.core.AbstractDockerCmdExecFactory; +import com.github.dockerjava.core.DockerClientConfig; +import com.github.dockerjava.core.SSLConfig; +import com.github.dockerjava.core.WebTarget; +import com.github.dockerjava.netty.NettyWebTarget; +import io.netty.bootstrap.Bootstrap; +import io.netty.channel.*; +import io.netty.channel.epoll.EpollDomainSocketChannel; +import io.netty.channel.epoll.EpollEventLoopGroup; +import io.netty.channel.kqueue.KQueueDomainSocketChannel; +import io.netty.channel.kqueue.KQueueEventLoopGroup; +import io.netty.channel.nio.NioEventLoopGroup; +import io.netty.channel.socket.DuplexChannel; +import io.netty.channel.socket.SocketChannel; +import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.channel.unix.DomainSocketAddress; +import io.netty.channel.unix.UnixChannel; +import io.netty.handler.codec.http.HttpClientCodec; +import io.netty.handler.ssl.SslHandler; +import io.netty.handler.timeout.IdleState; +import io.netty.handler.timeout.IdleStateEvent; +import io.netty.handler.timeout.IdleStateHandler; +import io.netty.util.concurrent.DefaultThreadFactory; +import org.apache.commons.lang.SystemUtils; +import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.testcontainers.DockerClientFactory; + +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLParameters; +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.security.Security; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; + +import static com.google.common.base.Preconditions.checkNotNull; + +/** + * This class is a modified version of docker-java's NettyDockerCmdExecFactory v3.1.0-rc-2 + * Changes: + * - daemonized thread factory + * - thread group + * - the logging handler removed + * - + */ +public class TestcontainersDockerCmdExecFactory extends AbstractDockerCmdExecFactory implements DockerCmdExecFactory { + + private static final String THREAD_PREFIX = "testcontainers-netty"; + + /* + * useful links: + * + * http://stackoverflow.com/questions/33296749/netty-connect-to-unix-domain-socket-failed + * http://netty.io/wiki/native-transports.html + * https://github.com/netty/netty/blob/master/example/src/main/java/io/netty/example/http/snoop/HttpSnoopClient.java + * https://github.com/slandelle/netty-request-chunking/blob/master/src/test/java/slandelle/ChunkingTest.java + */ + + private Bootstrap bootstrap; + + private EventLoopGroup eventLoopGroup; + + private NettyInitializer nettyInitializer; + + private WebTarget baseResource; + + private Integer connectTimeout = null; + + private Integer readTimeout = null; + + @Override + public void init(DockerClientConfig dockerClientConfig) { + super.init(dockerClientConfig); + + bootstrap = new Bootstrap(); + + String scheme = dockerClientConfig.getDockerHost().getScheme(); + + if ("unix".equals(scheme)) { + nettyInitializer = new UnixDomainSocketInitializer(); + } else if ("tcp".equals(scheme)) { + nettyInitializer = new InetSocketInitializer(); + } + + eventLoopGroup = nettyInitializer.init(bootstrap, dockerClientConfig); + + baseResource = new NettyWebTarget(this::connect).path(dockerClientConfig.getApiVersion().asWebPathPart()); + } + + private DuplexChannel connect() { + try { + return connect(bootstrap); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + private DuplexChannel connect(final Bootstrap bootstrap) throws InterruptedException { + return nettyInitializer.connect(bootstrap); + } + + private interface NettyInitializer { + EventLoopGroup init(final Bootstrap bootstrap, DockerClientConfig dockerClientConfig); + + DuplexChannel connect(final Bootstrap bootstrap) throws InterruptedException; + } + + private class UnixDomainSocketInitializer implements NettyInitializer { + @Override + public EventLoopGroup init(Bootstrap bootstrap, DockerClientConfig dockerClientConfig) { + if (SystemUtils.IS_OS_LINUX) { + return epollGroup(); + } else if (SystemUtils.IS_OS_MAC_OSX) { + return kqueueGroup(); + } + throw new RuntimeException("Unspported OS"); + } + + private ThreadFactory createThreadFactory() { + return new DefaultThreadFactory(THREAD_PREFIX, true, Thread.NORM_PRIORITY, DockerClientFactory.TESTCONTAINERS_THREAD_GROUP); + } + + public EventLoopGroup epollGroup() { + EventLoopGroup epollEventLoopGroup = new EpollEventLoopGroup(0, createThreadFactory()); + + ChannelFactory factory = () -> configure(new EpollDomainSocketChannel()); + + bootstrap.group(epollEventLoopGroup).channelFactory(factory).handler(new ChannelInitializer() { + @Override + protected void initChannel(final UnixChannel channel) throws Exception { + channel.pipeline().addLast(new HttpClientCodec()); + } + }); + return epollEventLoopGroup; + } + + public EventLoopGroup kqueueGroup() { + EventLoopGroup nioEventLoopGroup = new KQueueEventLoopGroup(0, createThreadFactory()); + + bootstrap.group(nioEventLoopGroup).channel(KQueueDomainSocketChannel.class) + .handler(new ChannelInitializer() { + @Override + protected void initChannel(final KQueueDomainSocketChannel channel) throws Exception { + channel.pipeline().addLast(new HttpClientCodec()); + } + }); + + return nioEventLoopGroup; + } + + @Override + public DuplexChannel connect(Bootstrap bootstrap) throws InterruptedException { + return (DuplexChannel) bootstrap.connect(new DomainSocketAddress("/var/run/docker.sock")).sync().channel(); + } + } + + private class InetSocketInitializer implements NettyInitializer { + @Override + public EventLoopGroup init(Bootstrap bootstrap, final DockerClientConfig dockerClientConfig) { + EventLoopGroup nioEventLoopGroup = new NioEventLoopGroup(0, new DefaultThreadFactory(THREAD_PREFIX)); + + // TODO do we really need BouncyCastle? + Security.addProvider(new BouncyCastleProvider()); + + ChannelFactory factory = () -> configure(new NioSocketChannel()); + + bootstrap.group(nioEventLoopGroup).channelFactory(factory) + .handler(new ChannelInitializer() { + @Override + protected void initChannel(final SocketChannel channel) throws Exception { + channel.pipeline().addLast(new HttpClientCodec()); + } + }); + + return nioEventLoopGroup; + } + + @Override + public DuplexChannel connect(Bootstrap bootstrap) throws InterruptedException { + DockerClientConfig dockerClientConfig = getDockerClientConfig(); + String host = dockerClientConfig.getDockerHost().getHost(); + int port = dockerClientConfig.getDockerHost().getPort(); + + if (port == -1) { + throw new RuntimeException("no port configured for " + host); + } + + DuplexChannel channel = (DuplexChannel) bootstrap.connect(host, port).sync().channel(); + + final SslHandler ssl = initSsl(dockerClientConfig); + + if (ssl != null) { + channel.pipeline().addFirst(ssl); + } + + return channel; + } + + private SslHandler initSsl(DockerClientConfig dockerClientConfig) { + SslHandler ssl = null; + + try { + String host = dockerClientConfig.getDockerHost().getHost(); + int port = dockerClientConfig.getDockerHost().getPort(); + + final SSLConfig sslConfig = dockerClientConfig.getSSLConfig(); + + if (sslConfig != null && sslConfig.getSSLContext() != null) { + + SSLEngine engine = sslConfig.getSSLContext().createSSLEngine(host, port); + engine.setUseClientMode(true); + engine.setSSLParameters(enableHostNameVerification(engine.getSSLParameters())); + + // in the future we may use HostnameVerifier like here: + // https://github.com/AsyncHttpClient/async-http-client/blob/1.8.x/src/main/java/com/ning/http/client/providers/netty/NettyConnectListener.java#L76 + + ssl = new SslHandler(engine); + } + + } catch (Exception e) { + throw new RuntimeException(e); + } + + return ssl; + } + } + + public SSLParameters enableHostNameVerification(SSLParameters sslParameters) { + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + return sslParameters; + } + + @Override + public void close() throws IOException { + checkNotNull(eventLoopGroup, "Factory not initialized. You probably forgot to call init()!"); + + eventLoopGroup.shutdownGracefully(); + } + + /** + * Configure connection timeout in milliseconds + */ + public TestcontainersDockerCmdExecFactory withConnectTimeout(Integer connectTimeout) { + this.connectTimeout = connectTimeout; + return this; + } + + /** + * Configure read timeout in milliseconds + */ + public TestcontainersDockerCmdExecFactory withReadTimeout(Integer readTimeout) { + this.readTimeout = readTimeout; + return this; + } + + private T configure(T channel) { + ChannelConfig channelConfig = channel.config(); + + if (connectTimeout != null) { + channelConfig.setConnectTimeoutMillis(connectTimeout); + } + if (readTimeout != null) { + channel.pipeline().addLast("readTimeoutHandler", new ReadTimeoutHandler()); + } + + return channel; + } + + private final class ReadTimeoutHandler extends IdleStateHandler { + private boolean alreadyTimedOut; + + ReadTimeoutHandler() { + super(readTimeout, 0, 0, TimeUnit.MILLISECONDS); + } + + /** + * Called when a read timeout was detected. + */ + @Override + protected synchronized void channelIdle(ChannelHandlerContext ctx, IdleStateEvent evt) throws Exception { + assert evt.state() == IdleState.READER_IDLE; + final Channel channel = ctx.channel(); + if (channel == null || !channel.isActive() || alreadyTimedOut) { + return; + } + DockerClientConfig dockerClientConfig = getDockerClientConfig(); + final Object dockerAPIEndpoint = dockerClientConfig.getDockerHost(); + final String msg = "Read timed out: No data received within " + readTimeout + + "ms. Perhaps the docker API (" + dockerAPIEndpoint + + ") is not responding normally, or perhaps you need to increase the readTimeout value."; + final Exception ex = new SocketTimeoutException(msg); + ctx.fireExceptionCaught(ex); + alreadyTimedOut = true; + } + } + + protected WebTarget getBaseResource() { + checkNotNull(baseResource, "Factory not initialized, baseResource not set. You probably forgot to call init()!"); + return baseResource; + } +} diff --git a/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java b/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java index 6d9427b61f3..71caa59566b 100644 --- a/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java +++ b/core/src/main/java/org/testcontainers/images/builder/ImageFromDockerfile.java @@ -39,7 +39,7 @@ public class ImageFromDockerfile extends LazyFuture implements private static final Set imagesToDelete = Sets.newConcurrentHashSet(); static { - Runtime.getRuntime().addShutdownHook(new Thread(() -> { + Runtime.getRuntime().addShutdownHook(new Thread(DockerClientFactory.TESTCONTAINERS_THREAD_GROUP, () -> { DockerClient dockerClientForCleaning = DockerClientFactory.instance().client(); try { for (String dockerImageName : imagesToDelete) { diff --git a/core/src/main/java/org/testcontainers/utility/MountableFile.java b/core/src/main/java/org/testcontainers/utility/MountableFile.java index 9f99fe064af..0dc4c1afaf8 100644 --- a/core/src/main/java/org/testcontainers/utility/MountableFile.java +++ b/core/src/main/java/org/testcontainers/utility/MountableFile.java @@ -8,6 +8,7 @@ import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; import org.apache.commons.lang.SystemUtils; import org.jetbrains.annotations.NotNull; +import org.testcontainers.DockerClientFactory; import org.testcontainers.images.builder.Transferable; import java.io.File; @@ -280,7 +281,7 @@ private void copyFromJarToLocation(final JarFile jarFile, } private void deleteOnExit(final Path path) { - Runtime.getRuntime().addShutdownHook(new Thread(() -> recursiveDeleteDir(path))); + Runtime.getRuntime().addShutdownHook(new Thread(DockerClientFactory.TESTCONTAINERS_THREAD_GROUP, () -> recursiveDeleteDir(path))); } /** diff --git a/core/src/main/java/org/testcontainers/utility/ResourceReaper.java b/core/src/main/java/org/testcontainers/utility/ResourceReaper.java index 4c56277b244..054b216dd19 100644 --- a/core/src/main/java/org/testcontainers/utility/ResourceReaper.java +++ b/core/src/main/java/org/testcontainers/utility/ResourceReaper.java @@ -112,6 +112,7 @@ public static String start(String hostIpAddress, DockerClient client, boolean wi } Thread kiraThread = new Thread( + DockerClientFactory.TESTCONTAINERS_THREAD_GROUP, () -> { while (true) { int index = 0; @@ -358,7 +359,7 @@ public void unregisterContainer(String identifier) { private void setHook() { if (hookIsSet.compareAndSet(false, true)) { // If the JVM stops without containers being stopped, try and stop the container. - Runtime.getRuntime().addShutdownHook(new Thread(this::performCleanup)); + Runtime.getRuntime().addShutdownHook(new Thread(DockerClientFactory.TESTCONTAINERS_THREAD_GROUP, this::performCleanup)); } } } diff --git a/core/src/test/java/org/testcontainers/DaemonTest.java b/core/src/test/java/org/testcontainers/DaemonTest.java new file mode 100644 index 00000000000..e339f30acc9 --- /dev/null +++ b/core/src/test/java/org/testcontainers/DaemonTest.java @@ -0,0 +1,61 @@ +package org.testcontainers; + +import org.junit.Test; +import org.rnorth.visibleassertions.VisibleAssertions; +import org.testcontainers.containers.GenericContainer; + +import java.io.File; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.junit.Assert.assertEquals; + +/** + * This test forks a new JVM, otherwise it's not possible to reliably diff the threads + */ +public class DaemonTest { + + public static void main(String[] args) { + Thread mainThread = Thread.currentThread(); + + GenericContainer genericContainer = null; + + try { + genericContainer = new GenericContainer().withCommand("top"); + genericContainer.start(); + + Set threads = new HashSet<>(Thread.getAllStackTraces().keySet()); + threads.remove(mainThread); + + Set nonDaemonThreads = threads.stream().filter(it -> !it.isDaemon()).collect(Collectors.toSet()); + + if (nonDaemonThreads.isEmpty()) { + VisibleAssertions.pass("All threads marked as daemon"); + } else { + String nonDaemonThreadNames = nonDaemonThreads.stream() + .map(Thread::getName) + .collect(Collectors.joining("\n", "\n", "")); + + VisibleAssertions.fail("Expected all threads to be daemons but the following are not:\n" + nonDaemonThreadNames); + } + } finally { + if (genericContainer != null) { + genericContainer.stop(); + } + } + } + + @Test + public void testThatAllThreadsAreDaemons() throws Exception { + ProcessBuilder processBuilder = new ProcessBuilder( + new File(System.getProperty("java.home")).toPath().resolve("bin").resolve("java").toString(), + "-ea", + "-classpath", + System.getProperty("java.class.path"), + DaemonTest.class.getCanonicalName() + ); + + assertEquals(0, processBuilder.inheritIO().start().waitFor()); + } +}