Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,20 @@ public FastTerminal(Callable<Terminal> builder, Consumer<Terminal> consumer) {
"fast-terminal-thread");
// a wedged builder must not keep the JVM alive; everything waits on the future, not the thread
this.buildThread.setDaemon(true);
}

/**
* Starts the build thread. Must be called <em>after</em> the caller has published this
* {@code FastTerminal} (e.g. assigned it to {@link MessageUtils#terminal}) so that code running
* on the build thread can obtain a non-null reference through {@link MessageUtils#getTerminal()}.
* <p>
* {@link Thread#start()} establishes a <em>happens-before</em> edge, so the assignment made by
* the caller before this method is visible to the build thread without additional
* synchronization.
*
* @see <a href="https://github.com/apache/maven/issues/12912">#12912</a>
*/
public void start() {
this.buildThread.start();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ public static void systemInstall() {
}

public static void systemInstall(Consumer<TerminalBuilder> builderConsumer, Consumer<Terminal> terminalConsumer) {
MessageUtils.terminal = new FastTerminal(
// Assign the FastTerminal to the field BEFORE starting the build thread so that code
// running on that thread (e.g. JLine's FFM provider init, logger calls) sees a non-null
// reference when it calls MessageUtils.getTerminal(). Thread.start() provides the
// happens-before edge that makes the assignment visible to the new thread.
FastTerminal ft = new FastTerminal(
() -> {
TerminalBuilder builder =
TerminalBuilder.builder().name("Maven").dumb(true);
Expand All @@ -64,6 +68,8 @@ public static void systemInstall(Consumer<TerminalBuilder> builderConsumer, Cons
terminalConsumer.accept(terminal);
}
});
MessageUtils.terminal = ft;
ft.start();
}

private static LineReader createReader(Terminal terminal) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,26 @@ void arbitraryTerminalMethodsFromTheConsumerDoNotDeadlock() {
});
}

/**
* Verifies that {@link MessageUtils#getTerminal()} is non-null when called from the builder
* callback. Before the fix, the {@code FastTerminal} constructor started its build thread
* before returning, so {@code MessageUtils.terminal} was still {@code null} when the build
* thread ran the builder callback &mdash; a race between the constructor returning and the
* thread scheduling. After the fix, {@code MessageUtils} assigns the field before calling
* {@link FastTerminal#start()}, and {@link Thread#start()} provides the happens-before edge.
*
* @see <a href="https://github.com/apache/maven/issues/12912">#12912</a>
*/
@Test
void terminalAssignmentIsVisibleFromBuilderCallback() {
assertTimeoutPreemptively(Duration.ofSeconds(30), () -> {
CompletableFuture<Terminal> observed = new CompletableFuture<>();
installAndAwait(builder -> observed.complete(MessageUtils.getTerminal()), terminal -> {});
assertNotNull(observed.get(), "MessageUtils.getTerminal() must not return null from the builder callback");
assertTrue(observed.get() instanceof FastTerminal, "terminal should be the FastTerminal wrapper");
});
}

/**
* Both terminal calls a single log statement makes, run on the terminal building thread.
*/
Expand Down
Loading