[arrow] branch main updated: GH-36340: [Java] Address race condition in allocator logger thread (#36341)

[li...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new 264b072e3f GH-36340: [Java] Address race condition in allocator logger thread (#36341)
264b072e3f is described below

commit 264b072e3fdf5f399b55a577fc7ca34255f664fa
Author: Laurent Goujon <la...@users.noreply.github.com>
AuthorDate: Wed Jun 28 05:14:18 2023 -0700

    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 apache/arrow#36340
    * Closes: #36340
    
    Authored-by: Laurent Goujon <la...@apache.org>
    Signed-off-by: David Li <li...@gmail.com>
---
 .../src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java    | 8 +++++---
 .../src/test/java/org/apache/arrow/memory/TestBaseAllocator.java  | 5 ++++-
 2 files changed, 9 insertions(+), 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
index 74b7a8530c..870114d7db 100644
--- 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 class PooledByteBufAllocatorL {
       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 class PooledByteBufAllocatorL {
     }
 
     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
index f0ec9ac378..7c0df0e98e 100644
--- 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.Arrays;
 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 class TestBaseAllocator {
           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);