Skip to content

Commit

Permalink
Stabilized LoggingPrintStreamTest
Browse files Browse the repository at this point in the history 
- yield helped a bit, but did not guarantee that all records will be processed
- close guarantees that.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
  • Loading branch information
@dmatej
dmatej committed Sep 17, 2022
1 parent 3194ca2 commit eaa4b06
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 180 deletions.
2 changes: 1 addition & 1 deletion ...sfish-jul-extension/src/main/java/org/glassfish/main/jul/handler/LogCollectorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class LogCollectorHandler extends Handler {
* @param loggerToFollow this handler will be added to this logger.
*/
public LogCollectorHandler(final Logger loggerToFollow) {
this.buffer = new LogRecordBuffer(100, 5);
buffer = new LogRecordBuffer(100, 5);
logger = loggerToFollow;
logger.addHandler(this);
}
Expand Down
181 changes: 3 additions & 178 deletions ...ssfish-jul-extension/src/main/java/org/glassfish/main/jul/handler/LoggingPrintStream.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.Formatter;
import java.util.Locale;
import java.util.logging.Level;
import java.util.logging.LogManager;
import java.util.logging.Logger;

/**
Expand All @@ -47,9 +46,7 @@
*/
public class LoggingPrintStream extends PrintStream {

private final LogManager logManager = LogManager.getLogManager();
private final ThreadLocal<StackTrace> perThreadStackTraces = new ThreadLocal<>();
private final Logger logger;

public static LoggingPrintStream create(final Logger logger, final Level level, final int bufferCapacity,
final Charset charset) {
Expand All @@ -60,7 +57,6 @@ public static LoggingPrintStream create(final Logger logger, final Level level,
private LoggingPrintStream(final Logger logger, final Level level, final int bufferCapacity,
final Charset charset) {
super(null, false, charset);
this.logger = logger;
this.out = new LoggingOutputStream(logger, level, bufferCapacity, charset);
}

Expand All @@ -72,10 +68,6 @@ private LoggingOutputStream getOutputStream() {

@Override
public void println(Object object) {
if (!checkLocks()) {
return;
}

if (object instanceof Throwable) {
getOutputStream().addRecord((Throwable) object);
StackTrace stackTrace = new StackTrace((Throwable) object);
Expand All @@ -84,7 +76,6 @@ public void println(Object object) {
// No special processing if it is not an exception.
println(String.valueOf(object));
}

}


Expand All @@ -102,7 +93,7 @@ public PrintStream printf(Locale locale, String str, Object... args) {

@Override
public PrintStream format(String format, Object... args) {
StringBuilder sb = new StringBuilder();
StringBuilder sb = new StringBuilder(format.length());
try (Formatter formatter = new Formatter(sb, Locale.getDefault())) {
formatter.format(format, args);
}
Expand All @@ -113,7 +104,7 @@ public PrintStream format(String format, Object... args) {

@Override
public PrintStream format(Locale locale, String format, Object... args) {
StringBuilder sb = new StringBuilder();
StringBuilder sb = new StringBuilder(format.length());
try (Formatter formatter = new Formatter(sb, locale)) {
formatter.format(format, args);
}
Expand All @@ -122,145 +113,14 @@ public PrintStream format(Locale locale, String format, Object... args) {
}


@Override
public void print(String x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(Object x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(boolean x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(double x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(char x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(int x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(long x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(float x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void print(char[] x) {
if (checkLocks()) {
super.print(x);
}
}


@Override
public void println(boolean x) {
if (checkLocks()) {
super.println(x);
}
}


@Override
public void println(char x) {
if (checkLocks()) {
super.println(x);
}
}

@Override
public void println(int x) {
if (checkLocks()) {
super.println(x);
}
}


@Override
public void println(long x) {
if (checkLocks()) {
super.println(x);
}
}


@Override
public void println(float x) {
if (checkLocks()) {
super.println(x);
}
}


@Override
public void println(double x) {
if (checkLocks()) {
super.println(x);
}
}


@Override
public void println(char[] x) {
if (checkLocks()) {
super.println(x);
}
}


@Override
public void println() {
// ignored as it would produce an emptz record.
// ignored as it would produce an empty record.
}


@Override
public void println(String str) {
if (!checkLocks()) {
return;
}

final StackTrace recentStacktrace = perThreadStackTraces.get();
if (recentStacktrace == null) {
super.println(str);
Expand Down Expand Up @@ -300,47 +160,12 @@ public void write(byte[] buf, int off, int len) {
}


@Override
public void write(int b) {
if (checkLocks()) {
super.write(b);
}
}


@Override
public String toString() {
return super.toString() + " using " + this.out.toString();
}


/**
* LoggingPrintStream class is to support the java System.err and System.out
* redirection to server.log file.
* <p>
* When Java IO is redirected and System.out.println(...) is invoked by a thread with
* LogManager or Logger(SYSTEMERR_LOGGER,SYSTEOUT_LOGGER) locked, all kind of dead
* locks among threads will happen.
* <p>
* These dead locks are easily reproduced when jvm system properties
* "-Djava.security.manager" and "-Djava.security.debug=access,failure" are defined.
* These dead locks are basically because each thread has its own sequence of
* acquiring lock objects(LogManager,Logger,FileHandler and SysLogHandler, the buffer
* inside LoggingPrintStream).
* <p>
* There is no obvious way to define the lock hierarchy and control the lock sequence;
* Trylock is not a strightforward solution either.Beside they both create heavy
* dependence on the detail implementation of JDK and Appserver.
* <p>
* This method(checkLocks) is to find which locks current thread has and
* LoggingPrintStream object will decide whether to continue to do printing or
* give ip up to avoid the dead lock.
*/
private boolean checkLocks() {
return !Thread.holdsLock(logger) && !Thread.holdsLock(logManager);
}


/**
* {@link StackTrace} keeps track of a throwable printed
* by a thread as a result of {@link Throwable#printStackTrace(PrintStream)}
Expand Down
3 changes: 2 additions & 1 deletion ...sh-jul-extension/src/test/java/org/glassfish/main/jul/handler/LoggingPrintStreamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ public void testStreamMethods() throws Exception {
assertSame(stream, stream.format("That %s", Level.ALL));
assertSame(stream, stream.format(Locale.GERMAN, "Number: %f.", 8.7f));

Thread.yield();
// The internal buffer is managed by a thread, so we need to be sure it processed all records.
stream.close();
List<String> list = handler.getAll(LogRecord::getMessage);
assertAll(
() -> assertThat(list,
Expand Down

0 comments on commit eaa4b06

Please sign in to comment.