You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by sl...@apache.org on 2026/08/25 00:34:25 UTC
(maven) branch mng-fasterminal-deadlock created (now 18b081ff7b)
This is an automated email from the ASF dual-hosted git repository.
slachiewicz pushed a change to branch mng-fasterminal-deadlock
in repository https://gitbox.apache.org/repos/asf/maven.git
at 18b081ff7b Fix #12761: do not wait for the terminal on the thread building it
This branch includes the following new commits:
new 18b081ff7b Fix #12761: do not wait for the terminal on the thread building it
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(maven) 01/01: Fix #12761: do not wait for the terminal on the thread building it
Posted by sl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
slachiewicz pushed a commit to branch mng-fasterminal-deadlock
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 18b081ff7b101789d3724988de92d1ca21a543d5
Author: Sylwester Lachiewicz <sl...@apache.org>
AuthorDate: Tue Aug 25 02:34:14 2026 +0200
Fix #12761: do not wait for the terminal on the thread building it
MessageUtils.systemInstall publishes the FastTerminal before the background
thread has built the real one, and every FastTerminal method delegates through
getTerminal(). Anything that thread logs is therefore rendered through a terminal
that same thread is still producing, and parks on its own future. There is no
lock cycle, so jstack reports no deadlock.
Both the builder callable and the consumer callback are inside that window: the
consumer runs before terminal.complete(term). Two distinct call paths reach it
from a single log statement, the level rendering via toAnsi and the log sink via
terminal.writer(), so guarding one call site is not enough. getTerminal() hands
the building thread a dumb stand-in instead, which unblocks every delegating
method at once. Do not narrow this to a plain-text fallback in the message
builder: the writer path would still park.
The stand-in writes to the System.err captured at construction, not the live one,
which the consumer replaces with a logging-backed stream that would feed the
output back into the logger it came from.
Rendering degrades to unstyled text for the duration of the build, which is what
a dumb terminal would produce anyway. The build thread also becomes a daemon, so
a wedged build cannot keep the JVM alive.
JLine 4.x initialises its native loader during provider probing and logs a
warning when a library candidate fails, which is what made this reachable after
the 3.30.6 to 4.3.1 bump in 4.0.0-rc-6. The failure itself is harmless, JLine
recovers by extracting the bundled library.
---
impl/maven-jline/pom.xml | 5 +
.../java/org/apache/maven/jline/FastTerminal.java | 86 +++++++++++++---
.../maven/jline/FastTerminalReentrancyTest.java | 108 +++++++++++++++++++++
3 files changed, 187 insertions(+), 12 deletions(-)
diff --git a/impl/maven-jline/pom.xml b/impl/maven-jline/pom.xml
index 9ec3e9ae0c..c9385e747e 100644
--- a/impl/maven-jline/pom.xml
+++ b/impl/maven-jline/pom.xml
@@ -80,5 +80,10 @@ under the License.
<groupId>org.jline</groupId>
<artifactId>jansi-core</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git a/impl/maven-jline/src/main/java/org/apache/maven/jline/FastTerminal.java b/impl/maven-jline/src/main/java/org/apache/maven/jline/FastTerminal.java
index 996a7d71eb..313d4b351e 100644
--- a/impl/maven-jline/src/main/java/org/apache/maven/jline/FastTerminal.java
+++ b/impl/maven-jline/src/main/java/org/apache/maven/jline/FastTerminal.java
@@ -23,6 +23,7 @@
import java.io.OutputStream;
import java.io.PrintWriter;
import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
import java.util.concurrent.Callable;
import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
@@ -36,6 +37,7 @@
import org.jline.terminal.Size;
import org.jline.terminal.Sized;
import org.jline.terminal.Terminal;
+import org.jline.terminal.impl.DumbTerminal;
import org.jline.terminal.spi.SystemStream;
import org.jline.terminal.spi.TerminalExt;
import org.jline.terminal.spi.TerminalProvider;
@@ -47,23 +49,50 @@ public class FastTerminal implements TerminalExt {
private final CompletableFuture<Terminal> terminal;
+ /**
+ * The thread running the builder and the consumer. Every method of this class delegates through
+ * {@link #getTerminal()}, so code running on this thread before the terminal is published would
+ * wait on the very future it is itself computing.
+ */
+ private final Thread buildThread;
+
+ /**
+ * Stand-in handed to {@link #buildThread} while the real terminal is still being built, so that
+ * logging from the builder or the consumer degrades to unstyled output instead of deadlocking.
+ * Created on demand, and only ever by {@link #buildThread}.
+ */
+ private Terminal reentrantFallback;
+
+ /**
+ * Captured before {@link #buildThread} starts, hence before the consumer swaps the system streams
+ * for logging-backed ones. Writing to the live {@code System.err} instead would feed the fallback
+ * output back into the logger it came from.
+ */
+ private final OutputStream fallbackOutput;
+
public FastTerminal(Callable<Terminal> builder, Consumer<Terminal> consumer) {
this.terminal = new CompletableFuture<>();
- new Thread(
- () -> {
- try {
- Terminal term = builder.call();
- consumer.accept(term);
- terminal.complete(term);
- } catch (Exception e) {
- terminal.completeExceptionally(new MavenException(e));
- }
- },
- "fast-terminal-thread")
- .start();
+ this.fallbackOutput = System.err;
+ this.buildThread = new Thread(
+ () -> {
+ try {
+ Terminal term = builder.call();
+ consumer.accept(term);
+ terminal.complete(term);
+ } catch (Exception e) {
+ terminal.completeExceptionally(new MavenException(e));
+ }
+ },
+ "fast-terminal-thread");
+ // a wedged builder must not keep the JVM alive; everything waits on the future, not the thread
+ this.buildThread.setDaemon(true);
+ this.buildThread.start();
}
public TerminalExt getTerminal() {
+ if (isBuildThreadWaitingOnItself()) {
+ return (TerminalExt) reentrantFallback();
+ }
try {
return (TerminalExt) terminal.get();
} catch (Exception e) {
@@ -71,6 +100,39 @@ public TerminalExt getTerminal() {
}
}
+ /**
+ * True when the caller is the thread building the terminal and the terminal is not published yet.
+ * Waiting for the future here would never return, since this thread is the one that completes it.
+ */
+ private boolean isBuildThreadWaitingOnItself() {
+ return Thread.currentThread() == buildThread && !isBuilt();
+ }
+
+ /**
+ * Whether the terminal has been published. {@link MessageUtils#systemUninstall()} waits for the
+ * build to finish, so a caller that must not block has to check this first.
+ */
+ boolean isBuilt() {
+ return terminal.isDone();
+ }
+
+ private Terminal reentrantFallback() {
+ // only buildThread reaches this, so no synchronization is needed
+ if (reentrantFallback == null) {
+ try {
+ reentrantFallback = new DumbTerminal(
+ "fast-terminal-fallback",
+ Terminal.TYPE_DUMB,
+ InputStream.nullInputStream(),
+ fallbackOutput,
+ StandardCharsets.UTF_8);
+ } catch (IOException e) {
+ throw new MavenException(e);
+ }
+ }
+ return reentrantFallback;
+ }
+
@Override
public String getName() {
return getTerminal().getName();
diff --git a/impl/maven-jline/src/test/java/org/apache/maven/jline/FastTerminalReentrancyTest.java b/impl/maven-jline/src/test/java/org/apache/maven/jline/FastTerminalReentrancyTest.java
new file mode 100644
index 0000000000..2bd1a6db23
--- /dev/null
+++ b/impl/maven-jline/src/test/java/org/apache/maven/jline/FastTerminalReentrancyTest.java
@@ -0,0 +1,108 @@
+/*
+ * 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.jline;
+
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.time.Duration;
+import java.util.concurrent.CompletableFuture;
+
+import org.jline.terminal.Terminal;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
+
+/**
+ * {@link MessageUtils#systemInstall} publishes the terminal before the background thread has built
+ * it, so anything that thread logs is rendered through a terminal that same thread is still
+ * producing. See <a href="https://github.com/apache/maven/issues/12761">#12761</a>.
+ * <p>
+ * The timeouts are preemptive on purpose: a regression parks the build thread forever, and only an
+ * abandoning timeout turns that into a red test rather than a hung fork.
+ */
+class FastTerminalReentrancyTest {
+
+ @AfterEach
+ void tearDown() {
+ // MessageUtils.terminal is process-global; leaving it set breaks every later test.
+ // On a regression the build thread never finishes and systemUninstall waits for it (see
+ // #11048), on the test thread and outside any timeout, so the fork would hang instead of
+ // reporting the failure. Leave the state dirty in that case; the run is lost either way.
+ if (MessageUtils.getTerminal() instanceof FastTerminal ft && !ft.isBuilt()) {
+ return;
+ }
+ MessageUtils.systemUninstall();
+ }
+
+ @Test
+ void renderingAMessageFromTheBuilderDoesNotDeadlock() {
+ assertTimeoutPreemptively(Duration.ofSeconds(30), () -> {
+ CompletableFuture<String> rendered = new CompletableFuture<>();
+ installAndAwait(builder ->
+ rendered.complete(MessageUtils.builder().warning("WARNING").build()));
+ // the stand-in reports itself as dumb, so the style is dropped rather than emitted blind
+ assertEquals("WARNING", rendered.get());
+ });
+ }
+
+ @Test
+ void takingTheWriterFromTheBuilderDoesNotDeadlock() {
+ assertTimeoutPreemptively(Duration.ofSeconds(30), () -> {
+ CompletableFuture<Object> writer = new CompletableFuture<>();
+ installAndAwait(
+ builder -> writer.complete(MessageUtils.getTerminal().writer()));
+ assertNotNull(writer.get());
+ });
+ }
+
+ @Test
+ void renderingAMessageFromTheConsumerDoesNotDeadlock() {
+ assertTimeoutPreemptively(Duration.ofSeconds(30), () -> {
+ CompletableFuture<String> rendered = new CompletableFuture<>();
+ // the consumer runs on the same thread, before the future is completed
+ installAndAwait(
+ builder -> {},
+ terminal -> rendered.complete(
+ MessageUtils.builder().warning("WARNING").build()));
+ assertEquals("WARNING", rendered.get());
+ });
+ }
+
+ private void installAndAwait(java.util.function.Consumer<org.jline.terminal.TerminalBuilder> onBuilder) {
+ installAndAwait(onBuilder, terminal -> {});
+ }
+
+ private void installAndAwait(
+ java.util.function.Consumer<org.jline.terminal.TerminalBuilder> onBuilder,
+ java.util.function.Consumer<Terminal> onTerminal) {
+ MessageUtils.systemInstall(
+ builder -> {
+ onBuilder.accept(builder);
+ builder.dumb(true)
+ .system(false)
+ .streams(InputStream.nullInputStream(), OutputStream.nullOutputStream());
+ },
+ onTerminal);
+ // let the build finish, so a failure here is the build failing rather than a leaked thread
+ ((FastTerminal) MessageUtils.getTerminal()).getTerminal();
+ }
+}