Skip to content

Commit ec032db

Browse files
davisusanibarlidavidm
authored andcommitted
GH-34338: [Java] Removing the automatic enabling of BaseAllocator.DEBUG on -ea (#36042)
### Rationale for this change To close #34338. ### What changes are included in this PR? Removing the automatic enabling of BaseAllocator.DEBUG on -ea ### Are these changes tested? Unit test added. ### Are there any user-facing changes? Yes. * Closes: #34338 Lead-authored-by: david dali susanibar arce <[email protected]> Co-authored-by: David Li <[email protected]> Signed-off-by: David Li <[email protected]>
1 parent 87e0696 commit ec032db

File tree

2 files changed

+56
-23
lines changed

2 files changed

+56
-23
lines changed

memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,10 @@ abstract class BaseAllocator extends Accountant implements BufferAllocator {
5353
if (propValue != null) {
5454
DEBUG = Boolean.parseBoolean(propValue);
5555
} else {
56-
DEBUG = AssertionUtil.isAssertionsEnabled();
56+
DEBUG = false;
5757
}
58-
logger.info("Debug mode " + (DEBUG ? "enabled." : "disabled."));
58+
logger.info("Debug mode " + (DEBUG ? "enabled."
59+
: "disabled. Enable with the VM option -Darrow.memory.debug.allocator=true."));
5960
}
6061

6162
public static final Config DEFAULT_CONFIG = ImmutableConfig.builder().build();

memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,32 +20,22 @@
2020
import static org.junit.Assert.assertArrayEquals;
2121
import static org.junit.Assert.assertEquals;
2222
import static org.junit.Assert.assertFalse;
23+
import static org.junit.Assert.assertThrows;
24+
import static org.junit.Assert.assertTrue;
2325

26+
import java.lang.reflect.Field;
27+
import java.lang.reflect.Modifier;
2428
import java.nio.ByteBuffer;
2529
import java.nio.ByteOrder;
2630
import java.util.Arrays;
2731

28-
import org.junit.AfterClass;
29-
import org.junit.BeforeClass;
3032
import org.junit.Test;
33+
import org.slf4j.LoggerFactory;
3134

32-
public class TestArrowBuf {
33-
34-
private static final int MAX_ALLOCATION = 8 * 1024;
35-
private static RootAllocator allocator;
35+
import ch.qos.logback.classic.Level;
36+
import ch.qos.logback.classic.Logger;
3637

37-
@BeforeClass
38-
public static void beforeClass() {
39-
allocator = new RootAllocator(MAX_ALLOCATION);
40-
}
41-
42-
/** Ensure the allocator is closed. */
43-
@AfterClass
44-
public static void afterClass() {
45-
if (allocator != null) {
46-
allocator.close();
47-
}
48-
}
38+
public class TestArrowBuf {
4939

5040
@Test(expected = IndexOutOfBoundsException.class)
5141
public void testSliceOutOfBoundsLength_RaisesIndexOutOfBoundsException() {
@@ -96,7 +86,8 @@ public void testSetBytesSliced() {
9686
expected[i] = (byte) i;
9787
}
9888
ByteBuffer data = ByteBuffer.wrap(expected);
99-
try (ArrowBuf buf = allocator.buffer(expected.length)) {
89+
try (BufferAllocator allocator = new RootAllocator(128);
90+
ArrowBuf buf = allocator.buffer(expected.length)) {
10091
buf.setBytes(0, data, 0, data.capacity());
10192

10293
byte[] actual = new byte[expected.length];
@@ -117,7 +108,8 @@ public void testSetBytesUnsliced() {
117108
int from = 10;
118109
int to = arrLength;
119110
byte[] expected = Arrays.copyOfRange(arr, from, to);
120-
try (ArrowBuf buf = allocator.buffer(expected.length)) {
111+
try (BufferAllocator allocator = new RootAllocator(128);
112+
ArrowBuf buf = allocator.buffer(expected.length)) {
121113
buf.setBytes(0, data, from, to - from);
122114

123115
byte[] actual = new byte[expected.length];
@@ -138,12 +130,52 @@ public void testSetBytesBigEndian() {
138130
assertFalse(data.hasArray());
139131
assertFalse(data.isDirect());
140132
assertEquals(ByteOrder.BIG_ENDIAN, data.order());
141-
try (ArrowBuf buf = allocator.buffer(expected.length)) {
133+
try (BufferAllocator allocator = new RootAllocator(128);
134+
ArrowBuf buf = allocator.buffer(expected.length)) {
142135
buf.setBytes(0, data);
143136
byte[] actual = new byte[expected.length];
144137
buf.getBytes(0, actual);
145138
assertArrayEquals(expected, actual);
146139
}
147140
}
148141

142+
@Test
143+
/**
144+
* Test that allocation history is not recorded even though
145+
* assertions are enabled in tests (GH-34338).
146+
*/
147+
public void testEnabledAssertion() {
148+
((Logger) LoggerFactory.getLogger("org.apache.arrow")).setLevel(Level.TRACE);
149+
try (BufferAllocator allocator = new RootAllocator(128)) {
150+
allocator.buffer(2);
151+
Exception e = assertThrows(IllegalStateException.class, () -> allocator.close());
152+
assertFalse(e.getMessage().contains("event log for:"));
153+
} finally {
154+
((Logger) LoggerFactory.getLogger("org.apache.arrow")).setLevel(null);
155+
}
156+
}
157+
158+
@Test
159+
public void testEnabledHistoricalLog() {
160+
((Logger) LoggerFactory.getLogger("org.apache.arrow")).setLevel(Level.TRACE);
161+
try {
162+
Field fieldDebug = BaseAllocator.class.getField("DEBUG");
163+
fieldDebug.setAccessible(true);
164+
Field modifiersDebug = Field.class.getDeclaredField("modifiers");
165+
modifiersDebug.setAccessible(true);
166+
modifiersDebug.setInt(fieldDebug, fieldDebug.getModifiers() & ~Modifier.FINAL);
167+
fieldDebug.set(null, true);
168+
try (BufferAllocator allocator = new RootAllocator(128)) {
169+
allocator.buffer(2);
170+
Exception e = assertThrows(IllegalStateException.class, () -> allocator.close());
171+
assertTrue(e.getMessage().contains("event log for:")); // JDK8, JDK11
172+
} finally {
173+
fieldDebug.set(null, false);
174+
}
175+
} catch (Exception e) {
176+
assertTrue(e.toString().contains("java.lang.NoSuchFieldException: modifiers")); // JDK17+
177+
} finally {
178+
((Logger) LoggerFactory.getLogger("org.apache.arrow")).setLevel(null);
179+
}
180+
}
149181
}

0 commit comments

Comments
 (0)