GH-36340: [Java] Address race condition in allocator logger thread (#…

…36341) ### Rationale for this change Address a race condition where the allocator logger thread starts logging before the class is fully initialized ### What changes are included in this PR? Change initialization order within the allocator to address the race condition. Add list of log messages captured during the test run if the assertion failed. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. Closes #36340 * Closes: #36340 Authored-by: Laurent Goujon <laurent@apache.org> Signed-off-by: David Li <li.davidm96@gmail.com>
apache · Jun 28, 2023 · 264b072 · 264b072
1 parent aa5592e
commit 264b072
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 4 deletions.
diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java b/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java
@@ -153,7 +153,7 @@ public InnerAllocator() {
       this.chunkSize = directArenas[0].chunkSize;
 
       if (memoryLogger.isTraceEnabled()) {
-        statusThread = new MemoryStatusThread();
+        statusThread = new MemoryStatusThread(this);
         statusThread.start();
       } else {
         statusThread = null;
@@ -256,16 +256,18 @@ public String toString() {
     }
 
     private class MemoryStatusThread extends Thread {
+      private final InnerAllocator allocator;
 
-      public MemoryStatusThread() {
+      public MemoryStatusThread(InnerAllocator allocator) {
         super("allocation.logger");
         this.setDaemon(true);
+        this.allocator = allocator;
       }
 
       @Override
       public void run() {
         while (true) {
-          memoryLogger.trace("Memory Usage: \n{}", PooledByteBufAllocatorL.this.allocator);
+          memoryLogger.trace("Memory Usage: \n{}", allocator);
           try {
             Thread.sleep(MEMORY_LOGGER_FREQUENCY_SECONDS * 1000);
           } catch (InterruptedException e) {

diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java
@@ -31,6 +31,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
+import java.util.stream.Collectors;
 
 import org.apache.arrow.memory.AllocationOutcomeDetails.Entry;
 import org.apache.arrow.memory.rounding.RoundingPolicy;
@@ -1123,7 +1124,9 @@ public void testMemoryUsage() {
           break;
         }
       }
-      assertTrue(result);
+      assertTrue("Log messages are:\n" +
+          memoryLogsAppender.list.stream().map(ILoggingEvent::toString).collect(Collectors.joining("\n")),
+          result);
     } finally {
       memoryLogsAppender.stop();
       logger.detachAppender(memoryLogsAppender);