Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Test From Hanging With AnimatedLabel Thread #597

Merged
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
86 changes: 66 additions & 20 deletions src/test/java/com/googlecode/lanterna/issue/Issue595.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,46 +18,92 @@
*/
package com.googlecode.lanterna.issue;

import com.googlecode.lanterna.gui2.AnimatedLabel;
import com.googlecode.lanterna.gui2.MultiWindowTextGUI;
import com.googlecode.lanterna.gui2.WindowBasedTextGUI;
import com.googlecode.lanterna.gui2.dialogs.WaitingDialog;
import com.googlecode.lanterna.screen.Screen;
import com.googlecode.lanterna.terminal.DefaultTerminalFactory;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

public class Issue595 {
public static void main(String... args) throws IOException {
public
class Issue595 {
private static final int SLEEP_SECONDS = 5;
private static final long SLEEP_MILLIS = SLEEP_SECONDS * 1000L;

public static
void main (String... args) throws IOException {
int exit_code = 0;
final DefaultTerminalFactory terminalFactory = new DefaultTerminalFactory();
final ExecutorService executorService = Executors.newSingleThreadExecutor();
try (final Screen screen = terminalFactory.createScreen()) {
screen.startScreen();
final WindowBasedTextGUI textGUI = new MultiWindowTextGUI(screen);

// POC
final WindowBasedTextGUI textGUI = new MultiWindowTextGUI(screen);
final WaitingDialog waitingDialog = WaitingDialog.createDialog("TITLE", "TEXT");
final ExecutorService executorService = Executors.newSingleThreadExecutor();
Thread.sleep(SLEEP_MILLIS);
waitingDialog.showDialog(textGUI, false);
CompletableFuture.runAsync(() -> {
try {
Thread.sleep(5000);
} catch (final InterruptedException e) {
Thread.currentThread().interrupt();
throw new RuntimeException(e);
} finally {
waitingDialog.close();
}
}, executorService)
.exceptionally(e -> {
throw new RuntimeException(e);
});
try {
Thread.sleep(SLEEP_MILLIS);
} catch (final InterruptedException e) {
Thread.currentThread()
.interrupt();
throw new RuntimeException(e);
} finally {
waitingDialog.close();
}
}, executorService)
.exceptionally(e -> {
throw new RuntimeException(e);
});
waitingDialog.waitUntilClosed();
System.out.println("WAIT DIALOG CLOSED");
} catch (final IOException e) {
throw new RuntimeException(e);

// Ensure Executor Thread Dead
executorService.shutdownNow();
if (!executorService.awaitTermination(SLEEP_SECONDS, TimeUnit.SECONDS)) {
throw new IllegalStateException("ExecutorService Unstoppable");
}

// Check for Animated Label Hanging Thread
final String animatedLabelName = AnimatedLabel.class.getSimpleName()
.toLowerCase();
final Optional<Thread> optionalAnimatedLabelThread = Thread.getAllStackTraces()
.keySet()
.stream()
.filter(thread -> thread.getName()
.toLowerCase()
.contains(animatedLabelName))
.findAny();
if (!optionalAnimatedLabelThread.isPresent()) {
return;
}
final Thread thread = optionalAnimatedLabelThread.get();
thread.join(SLEEP_MILLIS);
if (thread.isAlive()) {
throw new IllegalStateException("AnimatedLabel Thread Waiting");
}

} catch (final Throwable e) {
if (e instanceof InterruptedException) {
Thread.currentThread()
.interrupt();
}
++exit_code;
final StringWriter stringWriter = new StringWriter();
e.printStackTrace(new PrintWriter(stringWriter));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant, can just call e.printStackTrace()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things like e.printStackTrace() are often barfed at by code quality tools, like e.g. SonarQube.
The preferred way would be resorting to some logging-framework, so that it can be disabled
for production use - and not dump an exception-stackdump into user's face.

I'm aware that using a logging framework is not a simple decision, but the original code is already closer to a real solution than "simplifying" it back to e.printStackTrace().

My opinion: just leave that bit as it is. Simplifying it as suggested would be a step in the wrong direction.

Copy link
Author

@Kapral67 Kapral67 Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless we resort to a logging framework

StringWriter s = new StringWriter();
e.printStackTrace(new PrintWriter(s));
System.err.print(s);

Is equivalent to e.printStackTrace() (at least in vanilla JDK implementations)

This code is also part of the test suite, so users will not see the trace produced by potential exceptions here

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was copy+pasted from the bug report, I think :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, wrong PR

System.err.print(stringWriter);
} finally {
System.exit(exit_code);
}
executorService.shutdown();
}
}