[4.0.x] Fix #12761: do not wait for the terminal on the thread building it

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>

impl/maven-jline/src/main/java/org/apache/maven/jline/FastTerminal.java

@@ -21,6 +21,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.nio.charset.Charset;
 import java.util.concurrent.Callable;
@@ -47,20 +48,43 @@ 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;
+
+    /**
+     * Writer handed to {@link #buildThread} while the real terminal is still being built. Created on
+     * demand, and only ever by {@link #buildThread}.
+     */
+    private PrintWriter fallbackWriter;
+
+    /**
+     * 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() {
@@ -71,6 +95,30 @@ 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 PrintWriter fallbackWriter() {
+        // only buildThread reaches this, so no synchronization is needed
+        if (fallbackWriter == null) {
+            fallbackWriter = new PrintWriter(new OutputStreamWriter(fallbackOutput, Charset.defaultCharset()), true);
+        }
+        return fallbackWriter;
+    }
+
     @Override
     public String getName() {
         return getTerminal().getName();
@@ -93,7 +141,8 @@ public NonBlockingReader reader() {
 
     @Override
     public PrintWriter writer() {
-        return getTerminal().writer();
+        // the log sink writes here, and must not wait for the terminal this thread is building
+        return isBuildThreadWaitingOnItself() ? fallbackWriter() : getTerminal().writer();
     }
 
     @Override
@@ -198,7 +247,11 @@ public void flush() {
 
     @Override
     public String getType() {
-        return getTerminal().getType();
+        // AttributedCharSequence.toAnsi asks for the type first and renders plain for a dumb one,
+        // so answering here keeps message rendering off the terminal this thread is building
+        return isBuildThreadWaitingOnItself()
+                ? Terminal.TYPE_DUMB
+                : getTerminal().getType();
     }
 
     @Override

impl/maven-jline/src/test/java/org/apache/maven/jline/FastTerminalReentrancyTest.java

@@ -0,0 +1,107 @@
+/*
+ * 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 java.util.function.Consumer;
+
+import org.jline.terminal.Terminal;
+import org.jline.terminal.TerminalBuilder;
+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>
+ * A single log statement asks the terminal for two things, its type while rendering the message and
+ * its writer while emitting the line, so both are exercised from both halves of the window: the
+ * builder callable, and the consumer that runs before the terminal is published.
+ * <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 usingTheTerminalFromTheBuilderDoesNotDeadlock() {
+        assertTimeoutPreemptively(Duration.ofSeconds(30), () -> {
+            CompletableFuture<String[]> probed = new CompletableFuture<>();
+            installAndAwait(builder -> probed.complete(probe()), terminal -> {});
+            assertProbe(probed.get());
+        });
+    }
+
+    @Test
+    void usingTheTerminalFromTheConsumerDoesNotDeadlock() {
+        assertTimeoutPreemptively(Duration.ofSeconds(30), () -> {
+            CompletableFuture<String[]> probed = new CompletableFuture<>();
+            installAndAwait(builder -> {}, terminal -> probed.complete(probe()));
+            assertProbe(probed.get());
+        });
+    }
+
+    /**
+     * Both terminal calls a single log statement makes, run on the terminal building thread.
+     */
+    private static String[] probe() {
+        String rendered = MessageUtils.builder().warning("WARNING").build();
+        assertNotNull(MessageUtils.getTerminal().writer());
+        return new String[] {rendered, MessageUtils.getTerminal().getType()};
+    }
+
+    private static void assertProbe(String[] probed) {
+        // the stand-in reports itself dumb, so the style is dropped rather than emitted blind
+        assertEquals(Terminal.TYPE_DUMB, probed[1]);
+        assertEquals("WARNING", probed[0]);
+    }
+
+    private void installAndAwait(Consumer<TerminalBuilder> onBuilder, 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();
+    }
+}