Skip to content

Commit

Permalink
Merge pull request #445 from jamezp/LOGMGR-343-2.1
Browse files Browse the repository at this point in the history
[LOGMGR-343] If the AsyncHandler's worker thread and the current thre…
  • Loading branch information
jamezp authored Nov 10, 2023
2 parents 03a72b7 + 1e433f6 commit 2384521
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
7 changes: 7 additions & 0 deletions src/main/java/org/jboss/logmanager/handlers/AsyncHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ protected void doPublish(final ExtLogRecord record) {
// In case serialization is required by a child handler
record.getFormattedMessage();
}
if (Thread.currentThread() == thread) {
final Handler[] handlers = this.handlers;
for (Handler handler : handlers) {
handler.publish(record);
}
return;
}
if (overflowAction == OverflowAction.DISCARD) {
recordQueue.offer(record);
} else {
Expand Down
40 changes: 38 additions & 2 deletions src/test/java/org/jboss/logmanager/handlers/AsyncHandlerTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.concurrent.BlockingDeque;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.TimeUnit;
import java.util.logging.Handler;

import org.jboss.logmanager.AssertingErrorManager;
import org.jboss.logmanager.ExtHandler;
Expand Down Expand Up @@ -49,7 +50,7 @@ public void setup() {
handler = new BlockingQueueHandler();

asyncHandler = new AsyncHandler();
asyncHandler.setOverflowAction(OverflowAction.DISCARD);
asyncHandler.setOverflowAction(OverflowAction.BLOCK);
asyncHandler.addHandler(handler);
}

Expand Down Expand Up @@ -100,6 +101,37 @@ public void testMdc() throws Exception {
Assert.assertEquals(mdcValue, handler.getFirst());
}

@Test
public void reentry() throws Exception {
final ExtHandler reLogHandler = new ExtHandler() {
private final ThreadLocal<Boolean> entered = ThreadLocal.withInitial(() -> false);

@Override
protected void doPublish(final ExtLogRecord record) {
if (entered.get()) {
return;
}
try {
entered.set(true);
super.doPublish(record);
// Create a new record and act is if this was through a logger
asyncHandler.publish(createRecord());
} finally {
entered.set(false);
}
}
};
handler.addHandler(reLogHandler);
handler.setFormatter(new PatternFormatter("%s"));
asyncHandler.publish(createRecord());
// We should end up with two messages and a third should not happen
Assert.assertEquals("Test message", handler.getFirst());
Assert.assertEquals("Test message", handler.getFirst());
// This should time out. Then we end up with a null value. We could instead sleep for a shorter time and check
// if the queue is empty. However, a 5 second delay does not seem too long.
Assert.assertNull("Expected no more entries, but found " + handler.queue, handler.getFirst());
}

static ExtLogRecord createRecord() {
return new ExtLogRecord(Level.INFO, "Test message", AsyncHandlerTests.class.getName());
}
Expand All @@ -108,13 +140,17 @@ static class BlockingQueueHandler extends ExtHandler {
private final BlockingDeque<String> queue;

BlockingQueueHandler() {
queue = new LinkedBlockingDeque<String>();
queue = new LinkedBlockingDeque<>();
setErrorManager(AssertingErrorManager.of());
}

@Override
protected void doPublish(final ExtLogRecord record) {
queue.addLast(getFormatter().format(record));
final Handler[] handlers = this.handlers;
for (Handler handler : handlers) {
handler.publish(record);
}
super.doPublish(record);
}

Expand Down

0 comments on commit 2384521

Please sign in to comment.