Issue #838 Implement extension point to run diagnosis tools when forked JVM times out

maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java

@@ -882,6 +882,33 @@ public abstract class AbstractSurefireMojo extends AbstractMojo implements Suref
     @Parameter
     private Map<String, String> jdkToolchain;
 
+    /**
+     * Configuration map passed to every registered
+     * {@link org.apache.maven.surefire.extensions.ForkedProcessTimeoutExtension}
+     * via
+     * {@link org.apache.maven.surefire.extensions.ForkedProcessTimeoutContext#getExtensionContext()}.
+     * <br>
+     * Extension implementations read implementation-specific keys from this
+     * map. For instance the built-in jstack extension honors the
+     * {@code jstack.output.location} key (a directory path where the
+     * {@code surefire-timeout-jstack-*.txt} files are written).
+     * <br>
+     * Example:
+     * <pre>
+     * {@code
+     *    <configuration>
+     *        <forkedProcessTimeoutExtensionContext>
+     *            <jstack.output.location>${project.build.directory}/jstacks</jstack.output.location>
+     *        </forkedProcessTimeoutExtensionContext>
+     *    </configuration>
+     *    }
+     * </pre>
+     *
+     * @since 3.6.0
+     */
+    @Parameter
+    private Map<String, String> forkedProcessTimeoutExtensionContext;
+
     @Inject
     private ToolchainManager toolchainManager;
 
@@ -2371,7 +2398,8 @@ private ForkStarter createForkStarter(
                 forkConfiguration,
                 getForkedProcessTimeoutInSeconds(),
                 startupReportConfiguration,
-                log);
+                log,
+                forkedProcessTimeoutExtensionContext);
     }
 
     private InPluginVMSurefireStarter createInprocessStarter(

maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java

@@ -24,6 +24,8 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Queue;
 import java.util.Set;
@@ -53,6 +55,8 @@
 import org.apache.maven.plugin.surefire.booterclient.output.InPluginProcessDumpSingleton;
 import org.apache.maven.plugin.surefire.booterclient.output.NativeStdErrStreamConsumer;
 import org.apache.maven.plugin.surefire.booterclient.output.ThreadedStreamConsumer;
+import org.apache.maven.plugin.surefire.extensions.timeout.DefaultForkedProcessTimeoutContext;
+import org.apache.maven.plugin.surefire.extensions.timeout.TimeoutExtensionDispatcher;
 import org.apache.maven.plugin.surefire.log.api.ConsoleLogger;
 import org.apache.maven.plugin.surefire.report.DefaultReporterFactory;
 import org.apache.maven.plugin.surefire.report.ReportsMerger;
@@ -72,6 +76,7 @@
 import org.apache.maven.surefire.extensions.EventHandler;
 import org.apache.maven.surefire.extensions.ForkChannel;
 import org.apache.maven.surefire.extensions.ForkNodeFactory;
+import org.apache.maven.surefire.extensions.ForkedProcessTimeoutContext;
 import org.apache.maven.surefire.extensions.Stoppable;
 import org.apache.maven.surefire.extensions.util.CommandlineExecutor;
 import org.apache.maven.surefire.extensions.util.CommandlineStreams;
@@ -103,6 +108,7 @@
 import static org.apache.maven.surefire.api.util.internal.DaemonThreadFactory.newDaemonThreadFactory;
 import static org.apache.maven.surefire.api.util.internal.StringUtils.NL;
 import static org.apache.maven.surefire.booter.SystemPropertyManager.writePropertiesFile;
+import static org.apache.maven.surefire.booter.SystemUtils.pidOf;
 import static org.apache.maven.surefire.shared.utils.cli.ShutdownHookUtils.addShutDownHook;
 import static org.apache.maven.surefire.shared.utils.cli.ShutdownHookUtils.removeShutdownHook;
 
@@ -145,6 +151,10 @@ public class ForkStarter {
 
     private final int forkedProcessTimeoutInSeconds;
 
+    private final TimeoutExtensionDispatcher timeoutExtensionDispatcher;
+
+    private final Map<String, String> forkedProcessTimeoutExtensionContext;
+
     private final ProviderConfiguration providerConfiguration;
 
     private final StartupConfiguration startupConfiguration;
@@ -227,17 +237,39 @@ public ForkStarter(
             int forkedProcessTimeoutInSeconds,
             StartupReportConfiguration startupReportConfiguration,
             ConsoleLogger log) {
+        this(
+                providerConfiguration,
+                startupConfiguration,
+                forkConfiguration,
+                forkedProcessTimeoutInSeconds,
+                startupReportConfiguration,
+                log,
+                null);
+    }
+
+    public ForkStarter(
+            ProviderConfiguration providerConfiguration,
+            StartupConfiguration startupConfiguration,
+            ForkConfiguration forkConfiguration,
+            int forkedProcessTimeoutInSeconds,
+            StartupReportConfiguration startupReportConfiguration,
+            ConsoleLogger log,
+            Map<String, String> forkedProcessTimeoutExtensionContext) {
         this.forkConfiguration = forkConfiguration;
         this.providerConfiguration = providerConfiguration;
         this.forkedProcessTimeoutInSeconds = forkedProcessTimeoutInSeconds;
         this.startupConfiguration = startupConfiguration;
         this.startupReportConfiguration = startupReportConfiguration;
         this.log = log;
+        this.forkedProcessTimeoutExtensionContext = forkedProcessTimeoutExtensionContext == null
+                ? Collections.<String, String>emptyMap()
+                : Collections.unmodifiableMap(new HashMap<>(forkedProcessTimeoutExtensionContext));
         reportMerger = new DefaultReporterFactory(startupReportConfiguration, log);
         reportMerger.runStarting();
         defaultReporterFactories = new ConcurrentLinkedQueue<>();
         currentForkClients = new ConcurrentLinkedQueue<>();
         timeoutCheckScheduler = createTimeoutCheckScheduler();
+        timeoutExtensionDispatcher = new TimeoutExtensionDispatcher(log);
         triggerTimeoutCheck();
     }
 
@@ -252,6 +284,7 @@ public RunResult run(@Nonnull SurefireProperties effectiveSystemProperties, @Non
             reportMerger.close();
             pingThreadScheduler.shutdownNow();
             timeoutCheckScheduler.shutdownNow();
+            timeoutExtensionDispatcher.close();
             for (String line : logsAtEnd) {
                 log.warning(line);
             }
@@ -549,6 +582,7 @@ private RunResult fork(
         currentForkClients.add(forkClient);
         CountdownCloseable countdownCloseable =
                 new CountdownCloseable(eventConsumer, forkChannel.getCountdownCloseablePermits());
+        final ForkedProcessTimeoutContext[] timeoutContextHolder = new ForkedProcessTimeoutContext[1];
         try (CommandlineExecutor exec = new CommandlineExecutor(cli, countdownCloseable)) {
             forkChannel.tryConnectToClient();
             CommandlineStreams streams = exec.execute();
@@ -562,6 +596,21 @@ private RunResult fork(
 
             log.debug("Fork Channel [" + forkNumber + "] connected to the client.");
 
+            if (timeoutExtensionDispatcher.hasExtensions() && forkedProcessTimeoutInSeconds > 0) {
+                Long pid = pidOf(exec.getProcess());
+                File javaExecutable = cli.getExecutable() == null ? null : new File(cli.getExecutable());
+                final ForkedProcessTimeoutContext context = new DefaultForkedProcessTimeoutContext(
+                        pid == null ? -1L : pid,
+                        forkNumber,
+                        javaExecutable,
+                        startupReportConfiguration.getReportsDirectory(),
+                        forkedProcessTimeoutInSeconds,
+                        log,
+                        forkedProcessTimeoutExtensionContext);
+                timeoutContextHolder[0] = context;
+                forkClient.setTimeoutDetectedListener(() -> timeoutExtensionDispatcher.fireTimeoutDetected(context));
+            }
+
             result = exec.awaitExit();
 
             if (forkClient.hadTimeout()) {
@@ -595,6 +644,10 @@ private RunResult fork(
             }
             forkClient.close(runResult.isTimeout());
 
+            if (runResult.isTimeout() && timeoutContextHolder[0] != null) {
+                timeoutExtensionDispatcher.fireForkExited(timeoutContextHolder[0], runResult);
+            }
+
             if (!runResult.isTimeout()) {
                 Throwable cause = booterForkException == null ? null : booterForkException.getCause();
                 String detail = booterForkException == null ? "" : "\n" + booterForkException.getMessage();

maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java

@@ -87,6 +87,14 @@ public final class ForkClient implements EventHandler<Event> {
 
     private volatile StackTraceWriter errorInFork;
 
+    /**
+     * Optional callback invoked once from {@link #tryToTimeout} when the
+     * forked-process timeout has been reached, immediately before the KILL
+     * shutdown command is sent. Used by Surefire to dispatch
+     * {@code ForkedProcessTimeoutExtension#onTimeoutDetected} callbacks.
+     */
+    private volatile Runnable timeoutDetectedListener;
+
     public ForkClient(
             DefaultReporterFactory defaultReporterFactory, NotifiableTestStream notifiableTestStream, int forkNumber) {
         this.defaultReporterFactory = defaultReporterFactory;
@@ -116,6 +124,19 @@ public void setStopOnNextTestListener(ForkedProcessEventListener listener) {
         notifier.setStopOnNextTestListener(listener);
     }
 
+    /**
+     * Registers a one-shot callback invoked synchronously when the forked
+     * process timeout has been reached, immediately before the KILL command is
+     * sent. The forked JVM is still alive when the callback runs, so it may
+     * collect live diagnostics (for example a {@code jstack} dump).
+     *
+     * @param listener the callback to run, or {@code null} to clear; callback
+     *                 must not throw checked exceptions
+     */
+    public void setTimeoutDetectedListener(Runnable listener) {
+        this.timeoutDetectedListener = listener;
+    }
+
     private final class TestSetStartingListener implements ForkedProcessReportEventListener<TestSetReportEntry> {
         @Override
         public void handle(TestSetReportEntry reportEntry) {
@@ -288,8 +309,17 @@ public void tryToTimeout(long currentTimeMillis, int forkedProcessTimeoutInSecon
             final long forkedProcessTimeoutInMillis = 1000L * forkedProcessTimeoutInSeconds;
             final long startedAt = testSetStartedAt.get();
             if (startedAt > START_TIME_ZERO && currentTimeMillis - startedAt >= forkedProcessTimeoutInMillis) {
-                testSetStartedAt.set(START_TIME_NEGATIVE_TIMEOUT);
-                notifiableTestStream.shutdown(KILL);
+                if (testSetStartedAt.compareAndSet(startedAt, START_TIME_NEGATIVE_TIMEOUT)) {
+                    Runnable listener = timeoutDetectedListener;
+                    if (listener != null) {
+                        try {
+                            listener.run();
+                        } catch (RuntimeException ignored) {
+                            // listener failures must never prevent the kill
+                        }
+                    }
+                    notifiableTestStream.shutdown(KILL);
+                }
             }
         }
     }

maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/timeout/DefaultForkedProcessTimeoutContext.java

@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.plugin.surefire.extensions.timeout;
+
+import java.io.File;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.maven.plugin.surefire.log.api.ConsoleLogger;
+import org.apache.maven.surefire.extensions.ForkedProcessTimeoutContext;
+
+/**
+ * Default immutable {@link ForkedProcessTimeoutContext} implementation.
+ */
+public final class DefaultForkedProcessTimeoutContext implements ForkedProcessTimeoutContext {
+
+    private final long pid;
+    private final int forkNumber;
+    private final File javaExecutable;
+    private final File reportsDirectory;
+    private final int timeoutSeconds;
+    private final ConsoleLogger logger;
+    private final Map<String, String> extensionContext;
+
+    public DefaultForkedProcessTimeoutContext(
+            long pid,
+            int forkNumber,
+            File javaExecutable,
+            File reportsDirectory,
+            int timeoutSeconds,
+            ConsoleLogger logger) {
+        this(pid, forkNumber, javaExecutable, reportsDirectory, timeoutSeconds, logger, null);
+    }
+
+    public DefaultForkedProcessTimeoutContext(
+            long pid,
+            int forkNumber,
+            File javaExecutable,
+            File reportsDirectory,
+            int timeoutSeconds,
+            ConsoleLogger logger,
+            Map<String, String> extensionContext) {
+        this.pid = pid;
+        this.forkNumber = forkNumber;
+        this.javaExecutable = javaExecutable;
+        this.reportsDirectory = reportsDirectory;
+        this.timeoutSeconds = timeoutSeconds;
+        this.logger = logger;
+        this.extensionContext = extensionContext == null || extensionContext.isEmpty()
+                ? Collections.<String, String>emptyMap()
+                : Collections.unmodifiableMap(new HashMap<>(extensionContext));
+    }
+
+    @Override
+    public long getPid() {
+        return pid;
+    }
+
+    @Override
+    public int getForkNumber() {
+        return forkNumber;
+    }
+
+    @Override
+    public File getJavaExecutable() {
+        return javaExecutable;
+    }
+
+    @Override
+    public File getReportsDirectory() {
+        return reportsDirectory;
+    }
+
+    @Override
+    public int getTimeoutSeconds() {
+        return timeoutSeconds;
+    }
+
+    @Override
+    public ConsoleLogger getConsoleLogger() {
+        return logger;
+    }
+
+    @Override
+    public Map<String, String> getExtensionContext() {
+        return extensionContext;
+    }
+}