sparsick commented on code in PR #3372: URL: https://github.com/apache/maven-surefire/pull/3372#discussion_r3727582614
########## maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/timeout/JstackTimeoutExtension.java: ########## @@ -0,0 +1,202 @@ +/* + * 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.io.IOException; +import java.nio.file.Files; +import java.util.concurrent.TimeUnit; + +import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; +import org.apache.maven.surefire.api.suite.RunResult; +import org.apache.maven.surefire.extensions.ForkedProcessTimeoutContext; +import org.apache.maven.surefire.extensions.ForkedProcessTimeoutExtension; +import org.apache.maven.surefire.shared.lang3.SystemUtils; + +/** + * Built-in {@link ForkedProcessTimeoutExtension} that captures a {@code jstack} + * thread dump of the forked test JVM just before it is killed because of + * {@code forkedProcessTimeoutInSeconds}. + * <p> + * The output is written to + * {@code <reportsDirectory>/surefire-timeout-jstack-<forkNumber>-<pid>.txt}. + * <p> + * <strong>Activation:</strong> this extension is registered via + * {@code META-INF/services} but is <em>disabled by default</em>. To enable it + * set the system property {@code surefire.timeout.jstack.enabled=true} on the + * Maven process (for example with {@code MAVEN_OPTS} or + * {@code -Dsurefire.timeout.jstack.enabled=true}). The {@code jstack} binary + * is resolved from {@code ${java.home}/bin/jstack}, the parent JDK {@code bin} + * (Java 8 layout), {@code $JAVA_HOME/bin/jstack}, and finally from {@code PATH}. + * + * @since 3.6.0 + */ +public class JstackTimeoutExtension implements ForkedProcessTimeoutExtension { + + /** System property that enables this extension. */ + public static final String ENABLED_PROPERTY = "surefire.timeout.jstack.enabled"; + + /** + * Extension-context key that enables this extension from the POM via the + * {@code forkedProcessTimeoutExtensionContext} Mojo parameter. Set to + * {@code true} to enable. When either this key or {@link #ENABLED_PROPERTY} + * is set to {@code true}, the extension runs. + */ + public static final String ENABLED_KEY = "jstack.enabled"; + + /** + * Extension context key that overrides the directory where the + * {@code surefire-timeout-jstack-*.txt} files are written. When the key is + * missing or blank, the Surefire reports directory is used. + */ + public static final String OUTPUT_LOCATION_KEY = "jstack.output.location"; + + /** Wall-clock timeout for the jstack subprocess. */ + static final int JSTACK_TIMEOUT_SECONDS = 20; + + @Override + public void onTimeoutDetected(ForkedProcessTimeoutContext context) { + ConsoleLogger logger = context.getConsoleLogger(); + if (!isEnabled(context)) { + logger.debug("JstackTimeoutExtension disabled (set -D" + ENABLED_PROPERTY + "=true or POM key " + + ENABLED_KEY + "=true to enable)"); + return; + } + long pid = context.getPid(); + if (pid <= 0L) { + logger.warning("JstackTimeoutExtension: PID of forked JVM unknown (Java 8 or unsupported platform); " + + "skipping jstack for fork " + context.getForkNumber()); + return; + } + File jstack = resolveJstackBinary(); + if (jstack == null) { + logger.warning("JstackTimeoutExtension: cannot find jstack in java.home, JAVA_HOME or PATH; " + + "skipping jstack for fork " + context.getForkNumber() + " (pid=" + pid + ")"); + return; + } + File outputDirectory = resolveOutputDirectory(context, logger); Review Comment: question: what about using a default value for outputDirectory and log a warning instead of silent skipping? ########## maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/timeout/JstackTimeoutExtension.java: ########## @@ -0,0 +1,202 @@ +/* + * 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.io.IOException; +import java.nio.file.Files; +import java.util.concurrent.TimeUnit; + +import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; +import org.apache.maven.surefire.api.suite.RunResult; +import org.apache.maven.surefire.extensions.ForkedProcessTimeoutContext; +import org.apache.maven.surefire.extensions.ForkedProcessTimeoutExtension; +import org.apache.maven.surefire.shared.lang3.SystemUtils; + +/** + * Built-in {@link ForkedProcessTimeoutExtension} that captures a {@code jstack} + * thread dump of the forked test JVM just before it is killed because of + * {@code forkedProcessTimeoutInSeconds}. + * <p> + * The output is written to + * {@code <reportsDirectory>/surefire-timeout-jstack-<forkNumber>-<pid>.txt}. + * <p> + * <strong>Activation:</strong> this extension is registered via + * {@code META-INF/services} but is <em>disabled by default</em>. To enable it + * set the system property {@code surefire.timeout.jstack.enabled=true} on the + * Maven process (for example with {@code MAVEN_OPTS} or + * {@code -Dsurefire.timeout.jstack.enabled=true}). The {@code jstack} binary + * is resolved from {@code ${java.home}/bin/jstack}, the parent JDK {@code bin} + * (Java 8 layout), {@code $JAVA_HOME/bin/jstack}, and finally from {@code PATH}. + * + * @since 3.6.0 + */ +public class JstackTimeoutExtension implements ForkedProcessTimeoutExtension { + + /** System property that enables this extension. */ + public static final String ENABLED_PROPERTY = "surefire.timeout.jstack.enabled"; + + /** + * Extension-context key that enables this extension from the POM via the + * {@code forkedProcessTimeoutExtensionContext} Mojo parameter. Set to + * {@code true} to enable. When either this key or {@link #ENABLED_PROPERTY} + * is set to {@code true}, the extension runs. + */ + public static final String ENABLED_KEY = "jstack.enabled"; + + /** + * Extension context key that overrides the directory where the + * {@code surefire-timeout-jstack-*.txt} files are written. When the key is + * missing or blank, the Surefire reports directory is used. + */ + public static final String OUTPUT_LOCATION_KEY = "jstack.output.location"; + + /** Wall-clock timeout for the jstack subprocess. */ + static final int JSTACK_TIMEOUT_SECONDS = 20; + + @Override + public void onTimeoutDetected(ForkedProcessTimeoutContext context) { + ConsoleLogger logger = context.getConsoleLogger(); + if (!isEnabled(context)) { + logger.debug("JstackTimeoutExtension disabled (set -D" + ENABLED_PROPERTY + "=true or POM key " + + ENABLED_KEY + "=true to enable)"); + return; + } + long pid = context.getPid(); + if (pid <= 0L) { + logger.warning("JstackTimeoutExtension: PID of forked JVM unknown (Java 8 or unsupported platform); " + + "skipping jstack for fork " + context.getForkNumber()); + return; + } + File jstack = resolveJstackBinary(); + if (jstack == null) { Review Comment: question: If jstack is enabled but no jstack binary was found, would it be better to fail the build? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
