diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml index 8f286990453..b914b1fa10d 100644 --- a/java/memory/memory-core/pom.xml +++ b/java/memory/memory-core/pom.xml @@ -35,6 +35,10 @@ org.immutables value + + org.checkerframework + checker-qual + @@ -90,5 +94,46 @@ + + + checkerframework-jdk11+ + + [11,] + + + + + org.apache.maven.plugins + maven-compiler-plugin + + 8 + 8 + UTF-8 + + -Xmaxerrs + 10000 + -Xmaxwarns + 10000 + -AskipDefs=.*Test + -AatfDoNotCache + + + + org.checkerframework + checker + ${checker.framework.version} + + + + + org.immutables.value.internal.$processor$.$Processor + + org.checkerframework.checker.nullness.NullnessChecker + + + + + + diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java index 87769dd122d..b87f1345a51 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/Accountant.java @@ -22,6 +22,7 @@ import javax.annotation.concurrent.ThreadSafe; import org.apache.arrow.util.Preconditions; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Provides a concurrent way to manage account for memory usage without locking. Used as basis @@ -34,7 +35,7 @@ class Accountant implements AutoCloseable { /** * The parent allocator. */ - protected final Accountant parent; + protected final @Nullable Accountant parent; private final String name; @@ -59,7 +60,7 @@ class Accountant implements AutoCloseable { */ private final AtomicLong locallyHeldMemory = new AtomicLong(); - public Accountant(Accountant parent, String name, long reservation, long maxAllocation) { + public Accountant(@Nullable Accountant parent, String name, long reservation, long maxAllocation) { Preconditions.checkNotNull(name, "name must not be null"); Preconditions.checkArgument(reservation >= 0, "The initial reservation size must be non-negative."); Preconditions.checkArgument(maxAllocation >= 0, "The maximum allocation limit must be non-negative."); @@ -73,12 +74,13 @@ public Accountant(Accountant parent, String name, long reservation, long maxAllo this.allocationLimit.set(maxAllocation); if (reservation != 0) { + Preconditions.checkArgument(parent != null, "parent must not be null"); // we will allocate a reservation from our parent. final AllocationOutcome outcome = parent.allocateBytes(reservation); if (!outcome.isOk()) { throw new OutOfMemoryException(String.format( - "Failure trying to allocate initial reservation for Allocator. " + - "Attempted to allocate %d bytes.", reservation), outcome.getDetails()); + "Failure trying to allocate initial reservation for Allocator. " + + "Attempted to allocate %d bytes.", reservation), outcome.getDetails()); } } } @@ -103,7 +105,7 @@ AllocationOutcome allocateBytes(long size) { } } - private AllocationOutcome.Status allocateBytesInternal(long size, AllocationOutcomeDetails details) { + private AllocationOutcome.Status allocateBytesInternal(long size, @Nullable AllocationOutcomeDetails details) { final AllocationOutcome.Status status = allocate(size, true /*incomingUpdatePeek*/, false /*forceAllocation*/, details); if (!status.isOk()) { @@ -168,7 +170,7 @@ public boolean forceAllocate(long size) { * @return The outcome of the allocation. */ private AllocationOutcome.Status allocate(final long size, final boolean incomingUpdatePeak, - final boolean forceAllocation, AllocationOutcomeDetails details) { + final boolean forceAllocation, @Nullable AllocationOutcomeDetails details) { final long oldLocal = locallyHeldMemory.getAndAdd(size); final long newLocal = oldLocal + size; // Borrowed from Math.addExact (but avoid exception here) diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java index 3071c02f30a..6ccefdd9c16 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationManager.java @@ -18,6 +18,7 @@ package org.apache.arrow.memory; import org.apache.arrow.util.Preconditions; +import org.checkerframework.checker.nullness.qual.Nullable; /** * An AllocationManager is the implementation of a physical memory allocation. @@ -48,8 +49,9 @@ public abstract class AllocationManager { // This is mostly a semantic constraint on the API user: if the reference count reaches 0 in the owningLedger, then // there are not supposed to be any references through other allocators. In practice, this doesn't do anything // as the implementation just forces ownership to be transferred to one of the other extant references. - private volatile BufferLedger owningLedger; + private volatile @Nullable BufferLedger owningLedger; + @SuppressWarnings("nullness:method.invocation") //call to associate(a, b) not allowed on the given receiver protected AllocationManager(BufferAllocator accountingAllocator) { Preconditions.checkNotNull(accountingAllocator); accountingAllocator.assertOpen(); @@ -61,7 +63,7 @@ protected AllocationManager(BufferAllocator accountingAllocator) { this.owningLedger = associate(accountingAllocator, false); } - BufferLedger getOwningLedger() { + @Nullable BufferLedger getOwningLedger() { return owningLedger; } @@ -133,9 +135,9 @@ void release(final BufferLedger ledger) { // remove the mapping for the allocator // of calling BufferLedger Preconditions.checkState(map.containsKey(allocator), - "Expecting a mapping for allocator and reference manager"); + "Expecting a mapping for allocator and reference manager"); final BufferLedger oldLedger = map.remove(allocator); - + Preconditions.checkState(oldLedger != null, "Expecting a mapping for allocator and reference manager"); BufferAllocator oldAllocator = oldLedger.getAllocator(); if (oldAllocator instanceof BaseAllocator) { // needed for debug only: tell the allocator that AllocationManager is removing a @@ -168,7 +170,7 @@ void release(final BufferLedger ledger) { // the release call was made by a non-owning reference manager, so after remove there have // to be 1 or more mappings Preconditions.checkState(map.size() > 0, - "The final removal of reference manager should be connected to owning reference manager"); + "The final removal of reference manager should be connected to owning reference manager"); } } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java index 2977775e6ce..21a57eee49b 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcome.java @@ -19,15 +19,18 @@ import java.util.Optional; +import org.checkerframework.checker.nullness.qual.Nullable; + + /** * Describes the type of outcome that occurred when trying to account for allocation of memory. */ public class AllocationOutcome { private final Status status; - private final AllocationOutcomeDetails details; + private final @Nullable AllocationOutcomeDetails details; static final AllocationOutcome SUCCESS_INSTANCE = new AllocationOutcome(Status.SUCCESS); - AllocationOutcome(Status status, AllocationOutcomeDetails details) { + AllocationOutcome(Status status, @Nullable AllocationOutcomeDetails details) { this.status = status; this.details = details; } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java index 6499ce84b1a..3ceda71cce0 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/AllocationOutcomeDetails.java @@ -20,6 +20,9 @@ import java.util.ArrayDeque; import java.util.Deque; +import org.checkerframework.checker.nullness.qual.Nullable; + + /** * Captures details of allocation for each accountant in the hierarchical chain. */ @@ -47,7 +50,7 @@ void pushEntry(Accountant accountant, long totalUsedBeforeAllocation, long reque * Get the allocator that caused the failure. * @return the allocator that caused failure, null if there was no failure. */ - public BufferAllocator getFailedAllocator() { + public @Nullable BufferAllocator getFailedAllocator() { Entry top = allocEntries.peekLast(); if (top != null && top.allocationFailed && (top.accountant instanceof BufferAllocator)) { return (BufferAllocator) top.accountant; diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java index 2c2e93b2d70..112d36ece04 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java @@ -33,6 +33,7 @@ import org.apache.arrow.memory.util.MemoryUtil; import org.apache.arrow.util.Preconditions; import org.apache.arrow.util.VisibleForTesting; +import org.checkerframework.checker.nullness.qual.Nullable; /** * ArrowBuf serves as a facade over underlying memory by providing @@ -68,11 +69,11 @@ public final class ArrowBuf implements AutoCloseable { private static final int LOG_BYTES_PER_ROW = 10; private final long id = idGenerator.incrementAndGet(); private final ReferenceManager referenceManager; - private final BufferManager bufferManager; + private final @Nullable BufferManager bufferManager; private final long addr; private long readerIndex; private long writerIndex; - private final HistoricalLog historicalLog = BaseAllocator.DEBUG ? + private final @Nullable HistoricalLog historicalLog = BaseAllocator.DEBUG ? new HistoricalLog(BaseAllocator.DEBUG_LOG_LENGTH, "ArrowBuf[%d]", id) : null; private volatile long capacity; @@ -84,7 +85,7 @@ public final class ArrowBuf implements AutoCloseable { */ public ArrowBuf( final ReferenceManager referenceManager, - final BufferManager bufferManager, + final @Nullable BufferManager bufferManager, final long capacity, final long memoryAddress) { this.referenceManager = referenceManager; @@ -93,7 +94,7 @@ public ArrowBuf( this.capacity = capacity; this.readerIndex = 0; this.writerIndex = 0; - if (BaseAllocator.DEBUG) { + if (historicalLog != null) { historicalLog.recordEvent("create()"); } } @@ -244,7 +245,7 @@ public int hashCode() { } @Override - public boolean equals(Object obj) { + public boolean equals(@Nullable Object obj) { // identity equals only. return this == obj; } @@ -313,7 +314,7 @@ private void checkIndexD(long index, long fieldLength) { // check bounds Preconditions.checkArgument(fieldLength >= 0, "expecting non-negative data length"); if (index < 0 || index > capacity() - fieldLength) { - if (BaseAllocator.DEBUG) { + if (historicalLog != null) { historicalLog.logHistory(logger); } throw new IndexOutOfBoundsException(String.format( @@ -736,7 +737,7 @@ public void getBytes(long index, byte[] dst, int dstIndex, int length) { if (length != 0) { // copy "length" bytes from this ArrowBuf starting at addr(index) address // into dst byte array at dstIndex onwards - MemoryUtil.UNSAFE.copyMemory(null, addr(index), dst, MemoryUtil.BYTE_ARRAY_BASE_OFFSET + dstIndex, length); + MemoryUtil.copyMemory(null, addr(index), dst, MemoryUtil.BYTE_ARRAY_BASE_OFFSET + dstIndex, length); } } @@ -773,7 +774,7 @@ public void setBytes(long index, byte[] src, int srcIndex, long length) { if (length > 0) { // copy "length" bytes from src byte array at the starting index (srcIndex) // into this ArrowBuf starting at address "addr(index)" - MemoryUtil.UNSAFE.copyMemory(src, MemoryUtil.BYTE_ARRAY_BASE_OFFSET + srcIndex, null, addr(index), length); + MemoryUtil.copyMemory(src, MemoryUtil.BYTE_ARRAY_BASE_OFFSET + srcIndex, null, addr(index), length); } } @@ -799,7 +800,7 @@ public void getBytes(long index, ByteBuffer dst) { // at address srcAddress into the dst ByteBuffer starting at // address dstAddress final long dstAddress = MemoryUtil.getByteBufferAddress(dst) + dst.position(); - MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, dst.remaining()); + MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, dst.remaining()); // after copy, bump the next write position for the dst ByteBuffer dst.position(dst.position() + dst.remaining()); } else if (dst.hasArray()) { @@ -807,7 +808,7 @@ public void getBytes(long index, ByteBuffer dst) { // at address srcAddress into the dst ByteBuffer starting at // index dstIndex final int dstIndex = dst.arrayOffset() + dst.position(); - MemoryUtil.UNSAFE.copyMemory( + MemoryUtil.copyMemory( null, srcAddress, dst.array(), MemoryUtil.BYTE_ARRAY_BASE_OFFSET + dstIndex, dst.remaining()); // after copy, bump the next write position for the dst ByteBuffer dst.position(dst.position() + dst.remaining()); @@ -836,14 +837,14 @@ public void setBytes(long index, ByteBuffer src) { // copy src.remaining() bytes of data from src ByteBuffer starting at // address srcAddress into this ArrowBuf starting at address dstAddress final long srcAddress = MemoryUtil.getByteBufferAddress(src) + src.position(); - MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length); + MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length); // after copy, bump the next read position for the src ByteBuffer src.position(src.position() + length); } else if (src.hasArray()) { // copy src.remaining() bytes of data from src ByteBuffer starting at // index srcIndex into this ArrowBuf starting at address dstAddress final int srcIndex = src.arrayOffset() + src.position(); - MemoryUtil.UNSAFE.copyMemory( + MemoryUtil.copyMemory( src.array(), MemoryUtil.BYTE_ARRAY_BASE_OFFSET + srcIndex, null, dstAddress, length); // after copy, bump the next read position for the src ByteBuffer src.position(src.position() + length); @@ -896,7 +897,7 @@ public void setBytes(long index, ByteBuffer src, int srcIndex, int length) { // srcAddress into this ArrowBuf at address dstAddress final long srcAddress = MemoryUtil.getByteBufferAddress(src) + srcIndex; final long dstAddress = addr(index); - MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length); + MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length); } else { if (srcIndex == 0 && src.capacity() == length) { // copy the entire ByteBuffer from start to end of length @@ -936,7 +937,7 @@ public void getBytes(long index, ArrowBuf dst, long dstIndex, int length) { // dstAddress final long srcAddress = addr(index); final long dstAddress = dst.memoryAddress() + (long) dstIndex; - MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length); + MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length); } } @@ -966,7 +967,7 @@ public void setBytes(long index, ArrowBuf src, long srcIndex, long length) { // dstAddress final long srcAddress = src.memoryAddress() + srcIndex; final long dstAddress = addr(index); - MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length); + MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length); } } @@ -986,7 +987,7 @@ public void setBytes(long index, ArrowBuf src) { checkIndex(index, length); final long srcAddress = src.memoryAddress() + src.readerIndex; final long dstAddress = addr(index); - MemoryUtil.UNSAFE.copyMemory(null, srcAddress, null, dstAddress, length); + MemoryUtil.copyMemory(null, srcAddress, null, dstAddress, length); src.readerIndex(src.readerIndex + length); } @@ -1011,7 +1012,7 @@ public int setBytes(long index, InputStream in, int length) throws IOException { if (readBytes > 0) { // copy readBytes length of data from the tmp byte array starting // at srcIndex 0 into this ArrowBuf starting at address addr(index) - MemoryUtil.UNSAFE.copyMemory(tmp, MemoryUtil.BYTE_ARRAY_BASE_OFFSET, null, addr(index), readBytes); + MemoryUtil.copyMemory(tmp, MemoryUtil.BYTE_ARRAY_BASE_OFFSET, null, addr(index), readBytes); } } return readBytes; @@ -1033,7 +1034,7 @@ public void getBytes(long index, OutputStream out, int length) throws IOExceptio // copy length bytes of data from this ArrowBuf starting at // address addr(index) into the tmp byte array starting at index 0 byte[] tmp = new byte[length]; - MemoryUtil.UNSAFE.copyMemory(null, addr(index), tmp, MemoryUtil.BYTE_ARRAY_BASE_OFFSET, length); + MemoryUtil.copyMemory(null, addr(index), tmp, MemoryUtil.BYTE_ARRAY_BASE_OFFSET, length); // write the copied data to output stream out.write(tmp); } @@ -1109,7 +1110,7 @@ public long getId() { public void print(StringBuilder sb, int indent, Verbosity verbosity) { CommonUtil.indent(sb, indent).append(toString()); - if (BaseAllocator.DEBUG && verbosity.includeHistoricalLog) { + if (historicalLog != null && verbosity.includeHistoricalLog) { sb.append("\n"); historicalLog.buildHistory(sb, indent + 1, verbosity.includeStackTraces); } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java index 9337f48b743..8779c7a3434 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BaseAllocator.java @@ -30,6 +30,10 @@ import org.apache.arrow.memory.util.CommonUtil; import org.apache.arrow.memory.util.HistoricalLog; import org.apache.arrow.util.Preconditions; +import org.checkerframework.checker.initialization.qual.Initialized; +import org.checkerframework.checker.nullness.qual.KeyFor; +import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; import org.immutables.value.Value; /** @@ -64,17 +68,17 @@ abstract class BaseAllocator extends Accountant implements BufferAllocator { // Package exposed for sharing between AllocatorManger and BaseAllocator objects private final String name; private final RootAllocator root; - private final Object DEBUG_LOCK = DEBUG ? new Object() : null; + private final Object DEBUG_LOCK = new Object(); private final AllocationListener listener; - private final BaseAllocator parentAllocator; + private final @Nullable BaseAllocator parentAllocator; private final Map childAllocators; private final ArrowBuf empty; // members used purely for debugging - private final IdentityHashMap childLedgers; - private final IdentityHashMap reservations; - private final HistoricalLog historicalLog; + private final @Nullable IdentityHashMap childLedgers; + private final @Nullable IdentityHashMap reservations; + private final @Nullable HistoricalLog historicalLog; private final RoundingPolicy roundingPolicy; - private final AllocationManager.Factory allocationManagerFactory; + private final AllocationManager.@NonNull Factory allocationManagerFactory; private volatile boolean isClosed = false; // the allocator has been closed @@ -87,8 +91,10 @@ abstract class BaseAllocator extends Accountant implements BufferAllocator { * * @see Config */ + @SuppressWarnings({"nullness:method.invocation", "nullness:cast.unsafe"}) + //{"call to hist(,...) not allowed on the given receiver.", "cast cannot be statically verified"} protected BaseAllocator( - final BaseAllocator parentAllocator, + final @Nullable BaseAllocator parentAllocator, final String name, final Config config) throws OutOfMemoryException { super(parentAllocator, name, config.getInitReservation(), config.getMaxAllocation()); @@ -100,7 +106,7 @@ protected BaseAllocator( this.root = parentAllocator.root; empty = parentAllocator.empty; } else if (this instanceof RootAllocator) { - this.root = (RootAllocator) this; + this.root = (@Initialized RootAllocator) this; empty = createEmpty(); } else { throw new IllegalStateException("An parent allocator must either carry a root or be the " + @@ -131,7 +137,7 @@ public AllocationListener getListener() { } @Override - public BaseAllocator getParentAllocator() { + public @Nullable BaseAllocator getParentAllocator() { return parentAllocator; } @@ -187,7 +193,9 @@ void associateLedger(BufferLedger ledger) { assertOpen(); if (DEBUG) { synchronized (DEBUG_LOCK) { - childLedgers.put(ledger, null); + if (childLedgers != null) { + childLedgers.put(ledger, null); + } } } } @@ -201,6 +209,7 @@ void dissociateLedger(BufferLedger ledger) { assertOpen(); if (DEBUG) { synchronized (DEBUG_LOCK) { + Preconditions.checkState(childLedgers != null, "childLedgers must not be null"); if (!childLedgers.containsKey(ledger)) { throw new IllegalStateException("Trying to remove a child ledger that doesn't exist."); } @@ -223,7 +232,9 @@ private void childClosed(final BaseAllocator childAllocator) { synchronized (DEBUG_LOCK) { final Object object = childAllocators.remove(childAllocator); if (object == null) { - childAllocator.historicalLog.logHistory(logger); + if (childAllocator.historicalLog != null) { + childAllocator.historicalLog.logHistory(logger); + } throw new IllegalStateException("Child allocator[" + childAllocator.name + "] not found in parent allocator[" + name + "]'s childAllocators"); } @@ -280,12 +291,13 @@ public ArrowBuf buffer(final long initialRequestSize) { return buffer(initialRequestSize, null); } + @SuppressWarnings("nullness:dereference.of.nullable")//dereference of possibly-null reference allocationManagerFactory private ArrowBuf createEmpty() { return allocationManagerFactory.empty(); } @Override - public ArrowBuf buffer(final long initialRequestSize, BufferManager manager) { + public ArrowBuf buffer(final long initialRequestSize, @Nullable BufferManager manager) { assertOpen(); Preconditions.checkArgument(initialRequestSize >= 0, "the requested size must be non-negative"); @@ -332,7 +344,7 @@ public ArrowBuf buffer(final long initialRequestSize, BufferManager manager) { */ private ArrowBuf bufferWithoutReservation( final long size, - BufferManager bufferManager) throws OutOfMemoryException { + @Nullable BufferManager bufferManager) throws OutOfMemoryException { assertOpen(); final AllocationManager manager = newAllocationManager(size); @@ -388,8 +400,10 @@ public BufferAllocator newChildAllocator( if (DEBUG) { synchronized (DEBUG_LOCK) { childAllocators.put(childAllocator, childAllocator); - historicalLog.recordEvent("allocator[%s] created new child allocator[%s]", name, - childAllocator.getName()); + if (historicalLog != null) { + historicalLog.recordEvent("allocator[%s] created new child allocator[%s]", name, + childAllocator.getName()); + } } } else { childAllocators.put(childAllocator, childAllocator); @@ -439,14 +453,14 @@ public synchronized void close() { } // are there outstanding buffers? - final int allocatedCount = childLedgers.size(); + final int allocatedCount = childLedgers != null ? childLedgers.size() : 0; if (allocatedCount > 0) { throw new IllegalStateException( String.format("Allocator[%s] closed with outstanding buffers allocated (%d).\n%s", name, allocatedCount, toString())); } - if (reservations.size() != 0) { + if (reservations != null && reservations.size() != 0) { throw new IllegalStateException( String.format("Allocator[%s] closed with outstanding reservations (%d).\n%s", name, reservations.size(), @@ -486,7 +500,9 @@ public synchronized void close() { } if (DEBUG) { - historicalLog.recordEvent("closed"); + if (historicalLog != null) { + historicalLog.recordEvent("closed"); + } logger.debug(String.format("closed allocator[%s].", name)); } @@ -517,7 +533,9 @@ public String toVerboseString() { } private void hist(String noteFormat, Object... args) { - historicalLog.recordEvent(noteFormat, args); + if (historicalLog != null) { + historicalLog.recordEvent(noteFormat, args); + } } /** @@ -567,10 +585,14 @@ private void verifyAllocator( childTotal += Math.max(childAllocator.getAllocatedMemory(), childAllocator.reservation); } if (childTotal > getAllocatedMemory()) { - historicalLog.logHistory(logger); + if (historicalLog != null) { + historicalLog.logHistory(logger); + } logger.debug("allocator[" + name + "] child event logs BEGIN"); for (final BaseAllocator childAllocator : childSet) { - childAllocator.historicalLog.logHistory(logger); + if (childAllocator.historicalLog != null) { + childAllocator.historicalLog.logHistory(logger); + } } logger.debug("allocator[" + name + "] child event logs END"); throw new IllegalStateException( @@ -581,33 +603,39 @@ private void verifyAllocator( // Furthermore, the amount I've allocated should be that plus buffers I've allocated. long bufferTotal = 0; - final Set ledgerSet = childLedgers.keySet(); - for (final BufferLedger ledger : ledgerSet) { - if (!ledger.isOwningLedger()) { - continue; - } + final Set<@KeyFor("this.childLedgers") BufferLedger> ledgerSet = childLedgers != null ? + childLedgers.keySet() : null; + if (ledgerSet != null) { + for (final BufferLedger ledger : ledgerSet) { + if (!ledger.isOwningLedger()) { + continue; + } - final AllocationManager am = ledger.getAllocationManager(); - /* - * Even when shared, ArrowBufs are rewrapped, so we should never see the same instance - * twice. - */ - final BaseAllocator otherOwner = buffersSeen.get(am); - if (otherOwner != null) { - throw new IllegalStateException("This allocator's ArrowBuf already owned by another " + - "allocator"); - } - buffersSeen.put(am, this); + final AllocationManager am = ledger.getAllocationManager(); + /* + * Even when shared, ArrowBufs are rewrapped, so we should never see the same instance + * twice. + */ + final BaseAllocator otherOwner = buffersSeen.get(am); + if (otherOwner != null) { + throw new IllegalStateException("This allocator's ArrowBuf already owned by another " + + "allocator"); + } + buffersSeen.put(am, this); - bufferTotal += am.getSize(); + bufferTotal += am.getSize(); + } } // Preallocated space has to be accounted for - final Set reservationSet = reservations.keySet(); + final Set<@KeyFor("this.reservations") Reservation> reservationSet = reservations != null ? + reservations.keySet() : null; long reservedTotal = 0; - for (final Reservation reservation : reservationSet) { - if (!reservation.isUsed()) { - reservedTotal += reservation.getSize(); + if (reservationSet != null) { + for (final Reservation reservation : reservationSet) { + if (!reservation.isUsed()) { + reservedTotal += reservation.getSize(); + } } } @@ -644,9 +672,13 @@ private void verifyAllocator( if (reservedTotal != 0) { sb.append(String.format("reserved total : %d bytes.", reservedTotal)); - for (final Reservation reservation : reservationSet) { - reservation.historicalLog.buildHistory(sb, 0, true); - sb.append('\n'); + if (reservationSet != null) { + for (final Reservation reservation : reservationSet) { + if (reservation.historicalLog != null) { + reservation.historicalLog.buildHistory(sb, 0, true); + } + sb.append('\n'); + } } } @@ -689,16 +721,25 @@ void print(StringBuilder sb, int level, Verbosity verbosity) { child.print(sb, level + 2, verbosity); } - CommonUtil.indent(sb, level + 1).append(String.format("ledgers: %d\n", childLedgers.size())); - for (BufferLedger ledger : childLedgers.keySet()) { - ledger.print(sb, level + 2, verbosity); + CommonUtil.indent(sb, level + 1).append(String.format("ledgers: %d\n", childLedgers != null ? + childLedgers.size() : 0)); + if (childLedgers != null) { + for (BufferLedger ledger : childLedgers.keySet()) { + ledger.print(sb, level + 2, verbosity); + } } - final Set reservations = this.reservations.keySet(); - CommonUtil.indent(sb, level + 1).append(String.format("reservations: %d\n", reservations.size())); - for (final Reservation reservation : reservations) { - if (verbosity.includeHistoricalLog) { - reservation.historicalLog.buildHistory(sb, level + 3, true); + final Set<@KeyFor("this.reservations") Reservation> reservations = this.reservations != null ? + this.reservations.keySet() : null; + CommonUtil.indent(sb, level + 1).append(String.format("reservations: %d\n", + reservations != null ? reservations.size() : 0)); + if (reservations != null) { + for (final Reservation reservation : reservations) { + if (verbosity.includeHistoricalLog) { + if (reservation.historicalLog != null) { + reservation.historicalLog.buildHistory(sb, level + 3, true); + } + } } } @@ -706,17 +747,20 @@ void print(StringBuilder sb, int level, Verbosity verbosity) { } - private void dumpBuffers(final StringBuilder sb, final Set ledgerSet) { - for (final BufferLedger ledger : ledgerSet) { - if (!ledger.isOwningLedger()) { - continue; + private void dumpBuffers(final StringBuilder sb, + final @Nullable Set<@KeyFor("this.childLedgers") BufferLedger> ledgerSet) { + if (ledgerSet != null) { + for (final BufferLedger ledger : ledgerSet) { + if (!ledger.isOwningLedger()) { + continue; + } + final AllocationManager am = ledger.getAllocationManager(); + sb.append("UnsafeDirectLittleEndian[identityHashCode == "); + sb.append(Integer.toString(System.identityHashCode(am))); + sb.append("] size "); + sb.append(Long.toString(am.getSize())); + sb.append('\n'); } - final AllocationManager am = ledger.getAllocationManager(); - sb.append("UnsafeDirectLittleEndian[identityHashCode == "); - sb.append(Integer.toString(System.identityHashCode(am))); - sb.append("] size "); - sb.append(Long.toString(am.getSize())); - sb.append('\n'); } } @@ -813,7 +857,7 @@ RoundingPolicy getRoundingPolicy() { */ public class Reservation implements AllocationReservation { - private final HistoricalLog historicalLog; + private final @Nullable HistoricalLog historicalLog; private int nBytes = 0; private boolean used = false; private boolean closed = false; @@ -824,13 +868,16 @@ public class Reservation implements AllocationReservation { *

If {@linkplain #DEBUG} is true this will capture a historical * log of events relevant to this Reservation. */ + @SuppressWarnings("nullness:argument")//to handle null assignment on third party dependency: System.identityHashCode public Reservation() { if (DEBUG) { historicalLog = new HistoricalLog("Reservation[allocator[%s], %d]", name, System .identityHashCode(this)); historicalLog.recordEvent("created"); synchronized (DEBUG_LOCK) { - reservations.put(this, this); + if (reservations != null) { + reservations.put(this, this); + } } } else { historicalLog = null; @@ -901,7 +948,7 @@ public void close() { if (!isClosed()) { final Object object; synchronized (DEBUG_LOCK) { - object = reservations.remove(this); + object = reservations != null ? reservations.remove(this) : null; } if (object == null) { final StringBuilder sb = new StringBuilder(); @@ -911,7 +958,9 @@ public void close() { System.identityHashCode(this))); } - historicalLog.recordEvent("closed"); + if (historicalLog != null) { + historicalLog.recordEvent("closed"); + } } } @@ -928,7 +977,7 @@ public boolean reserve(int nBytes) { final AllocationOutcome outcome = BaseAllocator.this.allocateBytes(nBytes); - if (DEBUG) { + if (historicalLog != null) { historicalLog.recordEvent("reserve(%d) => %s", nBytes, Boolean.toString(outcome.isOk())); } @@ -959,7 +1008,7 @@ private ArrowBuf allocate(int nBytes) { final ArrowBuf arrowBuf = BaseAllocator.this.bufferWithoutReservation(nBytes, null); listener.onAllocation(nBytes); - if (DEBUG) { + if (historicalLog != null) { historicalLog.recordEvent("allocate() => %s", String.format("ArrowBuf[%d]", arrowBuf .getId())); } @@ -982,7 +1031,7 @@ private void releaseReservation(int nBytes) { releaseBytes(nBytes); - if (DEBUG) { + if (historicalLog != null) { historicalLog.recordEvent("releaseReservation(%d)", nBytes); } } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java index 90a4ef26fb4..c279e18f1ea 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferAllocator.java @@ -21,6 +21,7 @@ import org.apache.arrow.memory.rounding.DefaultRoundingPolicy; import org.apache.arrow.memory.rounding.RoundingPolicy; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Wrapper class to deal with byte buffer allocation. Ensures users only use designated methods. @@ -166,7 +167,7 @@ BufferAllocator newChildAllocator( * * @return parent allocator */ - BufferAllocator getParentAllocator(); + @Nullable BufferAllocator getParentAllocator(); /** * Returns the set of child allocators. diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java index 48b3e183d5a..1ca3e08ecf0 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/BufferLedger.java @@ -24,6 +24,7 @@ import org.apache.arrow.memory.util.CommonUtil; import org.apache.arrow.memory.util.HistoricalLog; import org.apache.arrow.util.Preconditions; +import org.checkerframework.checker.nullness.qual.Nullable; /** * The reference manager that binds an {@link AllocationManager} to @@ -32,7 +33,7 @@ * fate (same reference count). */ public class BufferLedger implements ValueWithKeyIncluded, ReferenceManager { - private final IdentityHashMap buffers = + private final @Nullable IdentityHashMap buffers = BaseAllocator.DEBUG ? new IdentityHashMap<>() : null; private static final AtomicLong LEDGER_ID_GENERATOR = new AtomicLong(0); // unique ID assigned to each ledger @@ -43,7 +44,7 @@ public class BufferLedger implements ValueWithKeyIncluded, Refe private final long lCreationTime = System.nanoTime(); private final BufferAllocator allocator; private final AllocationManager allocationManager; - private final HistoricalLog historicalLog = + private final @Nullable HistoricalLog historicalLog = BaseAllocator.DEBUG ? new HistoricalLog(BaseAllocator.DEBUG_LOG_LENGTH, "BufferLedger[%d]", 1) : null; private volatile long lDestructionTime = 0; @@ -122,7 +123,7 @@ public boolean release(int decrement) { "ref count decrement should be greater than or equal to 1"); // decrement the ref count final int refCnt = decrement(decrement); - if (BaseAllocator.DEBUG) { + if (historicalLog != null) { historicalLog.recordEvent("release(%d). original value: %d", decrement, refCnt + decrement); } @@ -178,7 +179,7 @@ public void retain() { @Override public void retain(int increment) { Preconditions.checkArgument(increment > 0, "retain(%s) argument is not positive", increment); - if (BaseAllocator.DEBUG) { + if (historicalLog != null) { historicalLog.recordEvent("retain(%d)", increment); } final int originalReferenceCount = bufRefCnt.getAndAdd(increment); @@ -233,20 +234,7 @@ public ArrowBuf deriveBuffer(final ArrowBuf sourceBuffer, long index, long lengt ); // logging - if (BaseAllocator.DEBUG) { - historicalLog.recordEvent( - "ArrowBuf(BufferLedger, BufferAllocator[%s], " + - "UnsafeDirectLittleEndian[identityHashCode == " + - "%d](%s)) => ledger hc == %d", - allocator.getName(), System.identityHashCode(derivedBuf), derivedBuf.toString(), - System.identityHashCode(this)); - - synchronized (buffers) { - buffers.put(derivedBuf, null); - } - } - - return derivedBuf; + return loggingArrowBufHistoricalLog(derivedBuf); } /** @@ -261,7 +249,7 @@ public ArrowBuf deriveBuffer(final ArrowBuf sourceBuffer, long index, long lengt * @return A new ArrowBuf that shares references with all ArrowBufs associated * with this BufferLedger */ - ArrowBuf newArrowBuf(final long length, final BufferManager manager) { + ArrowBuf newArrowBuf(final long length, final @Nullable BufferManager manager) { allocator.assertOpen(); // the start virtual address of the ArrowBuf will be same as address of memory chunk @@ -271,13 +259,17 @@ ArrowBuf newArrowBuf(final long length, final BufferManager manager) { final ArrowBuf buf = new ArrowBuf(this, manager, length, startAddress); // logging - if (BaseAllocator.DEBUG) { + return loggingArrowBufHistoricalLog(buf); + } + + private ArrowBuf loggingArrowBufHistoricalLog(ArrowBuf buf) { + if (historicalLog != null) { historicalLog.recordEvent( "ArrowBuf(BufferLedger, BufferAllocator[%s], " + "UnsafeDirectLittleEndian[identityHashCode == " + "%d](%s)) => ledger hc == %d", allocator.getName(), System.identityHashCode(buf), buf.toString(), System.identityHashCode(this)); - + Preconditions.checkState(buffers != null, "IdentityHashMap of buffers must not be null"); synchronized (buffers) { buffers.put(buf, null); } @@ -306,7 +298,7 @@ ArrowBuf newArrowBuf(final long length, final BufferManager manager) { @Override public ArrowBuf retain(final ArrowBuf srcBuffer, BufferAllocator target) { - if (BaseAllocator.DEBUG) { + if (historicalLog != null) { historicalLog.recordEvent("retain(%s)", target.getName()); } @@ -333,45 +325,48 @@ public ArrowBuf retain(final ArrowBuf srcBuffer, BufferAllocator target) { * @param targetReferenceManager The ledger to transfer ownership account to. * @return Whether transfer fit within target ledgers limits. */ - boolean transferBalance(final ReferenceManager targetReferenceManager) { + boolean transferBalance(final @Nullable ReferenceManager targetReferenceManager) { Preconditions.checkArgument(targetReferenceManager != null, - "Expecting valid target reference manager"); - final BufferAllocator targetAllocator = targetReferenceManager.getAllocator(); - Preconditions.checkArgument(allocator.getRoot() == targetAllocator.getRoot(), - "You can only transfer between two allocators that share the same root."); - - allocator.assertOpen(); - targetReferenceManager.getAllocator().assertOpen(); - - // if we're transferring to ourself, just return. - if (targetReferenceManager == this) { - return true; - } - - // since two balance transfers out from the allocation manager could cause incorrect - // accounting, we need to ensure - // that this won't happen by synchronizing on the allocation manager instance. - synchronized (allocationManager) { - if (allocationManager.getOwningLedger() != this) { - // since the calling reference manager is not the owning - // reference manager for the underlying memory, transfer is - // a NO-OP + "Expecting valid target reference manager"); + boolean overlimit = false; + if (targetReferenceManager != null) { + final BufferAllocator targetAllocator = targetReferenceManager.getAllocator(); + Preconditions.checkArgument(allocator.getRoot() == targetAllocator.getRoot(), + "You can only transfer between two allocators that share the same root."); + + allocator.assertOpen(); + targetReferenceManager.getAllocator().assertOpen(); + + // if we're transferring to ourself, just return. + if (targetReferenceManager == this) { return true; } - if (BaseAllocator.DEBUG) { - this.historicalLog.recordEvent("transferBalance(%s)", - targetReferenceManager.getAllocator().getName()); - } + // since two balance transfers out from the allocation manager could cause incorrect + // accounting, we need to ensure + // that this won't happen by synchronizing on the allocation manager instance. + synchronized (allocationManager) { + if (allocationManager.getOwningLedger() != this) { + // since the calling reference manager is not the owning + // reference manager for the underlying memory, transfer is + // a NO-OP + return true; + } - boolean overlimit = targetAllocator.forceAllocate(allocationManager.getSize()); - allocator.releaseBytes(allocationManager.getSize()); - // since the transfer can only happen from the owning reference manager, - // we need to set the target ref manager as the new owning ref manager - // for the chunk of memory in allocation manager - allocationManager.setOwningLedger((BufferLedger) targetReferenceManager); - return overlimit; + if (BaseAllocator.DEBUG && this.historicalLog != null) { + this.historicalLog.recordEvent("transferBalance(%s)", + targetReferenceManager.getAllocator().getName()); + } + + overlimit = targetAllocator.forceAllocate(allocationManager.getSize()); + allocator.releaseBytes(allocationManager.getSize()); + // since the transfer can only happen from the owning reference manager, + // we need to set the target ref manager as the new owning ref manager + // for the chunk of memory in allocation manager + allocationManager.setOwningLedger((BufferLedger) targetReferenceManager); + } } + return overlimit; } /** @@ -501,6 +496,7 @@ void print(StringBuilder sb, int indent, BaseAllocator.Verbosity verbosity) { if (!BaseAllocator.DEBUG) { sb.append("]\n"); } else { + Preconditions.checkArgument(buffers != null, "IdentityHashMap of buffers must not be null"); synchronized (buffers) { sb.append("] holds ") .append(buffers.size()) diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java index 15120c252fc..d57b72ba415 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java @@ -19,6 +19,9 @@ import java.lang.reflect.Field; +import org.checkerframework.checker.nullness.qual.Nullable; + + /** * A class for choosing the default allocation manager. */ @@ -39,7 +42,7 @@ public class DefaultAllocationManagerOption { /** * The default allocation manager factory. */ - private static AllocationManager.Factory DEFAULT_ALLOCATION_MANAGER_FACTORY = null; + private static AllocationManager.@Nullable Factory DEFAULT_ALLOCATION_MANAGER_FACTORY = null; /** * The allocation manager type. @@ -61,6 +64,7 @@ public enum AllocationManagerType { Unknown, } + @SuppressWarnings("nullness:argument") //enum types valueOf are implicitly non-null static AllocationManagerType getDefaultAllocationManagerType() { AllocationManagerType ret = AllocationManagerType.Unknown; @@ -103,6 +107,9 @@ static AllocationManager.Factory getDefaultAllocationManagerFactory() { return DEFAULT_ALLOCATION_MANAGER_FACTORY; } + @SuppressWarnings({"nullness:argument", "nullness:return"}) + //incompatible argument for parameter obj of Field.get + // Static member qualifying type may not be annotated private static AllocationManager.Factory getFactory(String clazzName) { try { Field field = Class.forName(clazzName).getDeclaredField("FACTORY"); diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java index edfa82392a4..740233ef411 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/LowCostIdentityHashMap.java @@ -19,6 +19,9 @@ import org.apache.arrow.util.Preconditions; import org.apache.arrow.util.VisibleForTesting; +import org.checkerframework.checker.initialization.qual.Initialized; +import org.checkerframework.checker.initialization.qual.UnderInitialization; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Highly specialized IdentityHashMap that implements only partial @@ -35,7 +38,7 @@ public class LowCostIdentityHashMap> { /* * The internal data structure to hold values. */ - private Object[] elementData; + private @Nullable Object [] elementData; // elementData[index] = null; /* Actual number of values. */ private int size; @@ -69,19 +72,20 @@ public LowCostIdentityHashMap(int maxSize) { if (maxSize >= 0) { this.size = 0; threshold = getThreshold(maxSize); - elementData = newElementArray(computeElementArraySize()); + elementData = newElementArrayUnderInitialized(computeElementArraySize()); } else { throw new IllegalArgumentException(); } } - private int getThreshold(int maxSize) { + private int getThreshold(@UnderInitialization LowCostIdentityHashMap this, + int maxSize) { // assign the threshold to maxSize initially, this will change to a // higher value if rehashing occurs. return maxSize > 2 ? maxSize : 2; } - private int computeElementArraySize() { + private int computeElementArraySize(@UnderInitialization LowCostIdentityHashMap this) { int arraySize = (int) (((long) threshold * 10000) / LOAD_FACTOR); // ensure arraySize is positive, the above cast from long to int type // leads to overflow and negative arraySize if threshold is too big @@ -95,7 +99,18 @@ private int computeElementArraySize() { * the number of elements * @return Reference to the element array */ - private Object[] newElementArray(int s) { + private Object[] newElementArrayInitialized(@Initialized LowCostIdentityHashMap this, int s) { + return new Object[s]; + } + + /** + * Create a new element array. + * + * @param s + * the number of elements + * @return Reference to the element array + */ + private Object[] newElementArrayUnderInitialized(@UnderInitialization LowCostIdentityHashMap this, int s) { return new Object[s]; } @@ -152,7 +167,7 @@ public boolean containsValue(V value) { * @param key the key. * @return the value of the mapping with the specified key. */ - public V get(K key) { + public @Nullable V get(K key) { Preconditions.checkNotNull(key); int index = findIndex(key, elementData); @@ -166,7 +181,7 @@ public V get(K key) { * empty spot if the key is not found in this table. */ @VisibleForTesting - int findIndex(Object key, Object[] array) { + int findIndex(@Nullable Object key, @Nullable Object[] array) { int length = array.length; int index = getModuloHash(key, length); int last = (index + length - 1) % length; @@ -184,7 +199,7 @@ int findIndex(Object key, Object[] array) { } @VisibleForTesting - static int getModuloHash(Object key, int length) { + static int getModuloHash(@Nullable Object key, int length) { return ((System.identityHashCode(key) & 0x7FFFFFFF) % length); } @@ -226,7 +241,7 @@ void rehash() { if (newlength == 0) { newlength = 1; } - Object[] newData = newElementArray(newlength); + @Nullable Object[] newData = newElementArrayInitialized(newlength); for (int i = 0; i < elementData.length; i++) { Object key = (elementData[i] == null) ? null : ((V) elementData[i]).getKey(); if (key != null) { @@ -250,7 +265,7 @@ private void computeMaxSize() { * @return the value of the removed mapping, or {@code null} if no mapping * for the specified key was found. */ - public V remove(K key) { + public @Nullable V remove(K key) { Preconditions.checkNotNull(key); boolean hashedOk; @@ -325,7 +340,7 @@ public int size() { * * @return next available value or null if none available */ - public V getNextValue() { + public @Nullable V getNextValue() { for (int i = 0; i < elementData.length; i++) { if (elementData[i] != null) { return (V) elementData[i]; diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java index fa1cfbdb293..b41576847d6 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/ArrowBufPointer.java @@ -21,6 +21,7 @@ import org.apache.arrow.memory.util.hash.ArrowBufHasher; import org.apache.arrow.memory.util.hash.SimpleHasher; import org.apache.arrow.util.Preconditions; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Pointer to a memory region within an {@link ArrowBuf}. @@ -33,7 +34,7 @@ public final class ArrowBufPointer { */ public static final int NULL_HASH_CODE = 0; - private ArrowBuf buf; + private @Nullable ArrowBuf buf; private long offset; @@ -62,6 +63,7 @@ public ArrowBufPointer() { public ArrowBufPointer(ArrowBufHasher hasher) { Preconditions.checkNotNull(hasher); this.hasher = hasher; + this.buf = null; } /** @@ -93,6 +95,7 @@ public ArrowBufPointer(ArrowBuf buf, long offset, long length, ArrowBufHasher ha * @param offset the start off set of the memory region pointed to. * @param length the length off set of the memory region pointed to. */ + public void set(ArrowBuf buf, long offset, long length) { this.buf = buf; this.offset = offset; @@ -105,7 +108,7 @@ public void set(ArrowBuf buf, long offset, long length) { * Gets the underlying buffer, or null if the underlying data is invalid or null. * @return the underlying buffer, if any, or null if the underlying data is invalid or null. */ - public ArrowBuf getBuf() { + public @Nullable ArrowBuf getBuf() { return buf; } @@ -118,7 +121,7 @@ public long getLength() { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) { return true; } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java index f02539a8a3d..21f063c939e 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/HistoricalLog.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.LinkedList; +import org.checkerframework.checker.nullness.qual.Nullable; import org.slf4j.Logger; /** @@ -32,7 +33,7 @@ public class HistoricalLog { private final LinkedList history = new LinkedList<>(); private final String idString; // the formatted id string private final int limit; // the limit on the number of events kept - private Event firstEvent; // the first stack trace recorded + private @Nullable Event firstEvent; // the first stack trace recorded /** * Constructor. The format string will be formatted and have its arguments @@ -68,6 +69,7 @@ public HistoricalLog(final String idStringFormat, Object... args) { public HistoricalLog(final int limit, final String idStringFormat, Object... args) { this.limit = limit; this.idString = String.format(idStringFormat, args); + this.firstEvent = null; } /** @@ -122,13 +124,16 @@ public synchronized void buildHistory( .append('\n'); if (firstEvent != null) { + long time = firstEvent.time; + String note = firstEvent.note; + final StackTrace stackTrace = firstEvent.stackTrace; sb.append(innerIndentation) - .append(firstEvent.time) + .append(time) .append(' ') - .append(firstEvent.note) + .append(note) .append('\n'); if (includeStackTrace) { - firstEvent.stackTrace.writeToBuilder(sb, indent + 2); + stackTrace.writeToBuilder(sb, indent + 2); } for (final Event event : history) { diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java index cc615c5b383..f79cf795312 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java @@ -25,15 +25,18 @@ import java.security.AccessController; import java.security.PrivilegedAction; +import org.checkerframework.checker.nullness.qual.Nullable; + import sun.misc.Unsafe; + /** * Utilities for memory related operations. */ public class MemoryUtil { private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(MemoryUtil.class); - private static final Constructor DIRECT_BUFFER_CONSTRUCTOR; + private static final @Nullable Constructor DIRECT_BUFFER_CONSTRUCTOR; /** * The unsafe object from which to access the off-heap memory. */ @@ -63,6 +66,9 @@ public class MemoryUtil { // try to get the unsafe object final Object maybeUnsafe = AccessController.doPrivileged(new PrivilegedAction() { @Override + @SuppressWarnings({"nullness:argument", "nullness:return"}) + // incompatible argument for parameter obj of Field.get + // incompatible types in return public Object run() { try { final Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe"); @@ -179,4 +185,11 @@ public static ByteBuffer directBuffer(long address, int capacity) { throw new UnsupportedOperationException( "sun.misc.Unsafe or java.nio.DirectByteBuffer.(long, int) not available"); } + + @SuppressWarnings("nullness:argument") //to handle null assignment on third party dependency: Unsafe + public static void copyMemory(@Nullable Object srcBase, long srcOffset, + @Nullable Object destBase, long destOffset, + long bytes) { + UNSAFE.copyMemory(srcBase, srcOffset, destBase, destOffset, bytes); + } } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java index cd864eb9985..a533edd7937 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/StackTrace.java @@ -19,12 +19,15 @@ import java.util.Arrays; +import org.checkerframework.checker.nullness.qual.Nullable; + + /** * Convenient way of obtaining and manipulating stack traces for debugging. */ public class StackTrace { - private final StackTraceElement[] stackTraceElements; + private final @Nullable StackTraceElement [] stackTraceElements; /** * Constructor. Captures the current stack trace. @@ -48,16 +51,18 @@ public void writeToBuilder(final StringBuilder sb, final int indent) { // write the stack trace in standard Java format for (StackTraceElement ste : stackTraceElements) { - sb.append(indentation) - .append("at ") - .append(ste.getClassName()) - .append('.') - .append(ste.getMethodName()) - .append('(') - .append(ste.getFileName()) - .append(':') - .append(Integer.toString(ste.getLineNumber())) - .append(")\n"); + if (ste != null) { + sb.append(indentation) + .append("at ") + .append(ste.getClassName()) + .append('.') + .append(ste.getMethodName()) + .append('(') + .append(ste.getFileName()) + .append(':') + .append(Integer.toString(ste.getLineNumber())) + .append(")\n"); + } } } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java index 75fc3f0c458..5de98d23bb8 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/MurmurHasher.java @@ -19,6 +19,7 @@ import org.apache.arrow.memory.ArrowBuf; import org.apache.arrow.memory.util.MemoryUtil; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Implementation of the Murmur hashing algorithm. @@ -157,7 +158,7 @@ private static int rotateLeft(int value, int count) { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) { return true; } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java index da0ee482997..3bf3c2a8283 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/hash/SimpleHasher.java @@ -20,6 +20,7 @@ import org.apache.arrow.memory.ArrowBuf; import org.apache.arrow.memory.util.MemoryUtil; +import org.checkerframework.checker.nullness.qual.Nullable; /** * A simple hasher that calculates the hash code of integers as is, @@ -110,7 +111,7 @@ public int hashCode() { } @Override - public boolean equals(Object obj) { + public boolean equals(@Nullable Object obj) { return obj != null && (obj instanceof SimpleHasher); } } diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java b/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java index 0ffc9447e43..8083033007d 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/util/Preconditions.java @@ -33,6 +33,8 @@ package org.apache.arrow.util; +import org.checkerframework.dataflow.qual.AssertMethod; + /** * Static convenience methods that help a method or constructor check whether it was invoked * correctly (whether its preconditions have been met). These methods generally accept a @@ -117,6 +119,7 @@ private Preconditions() {} * @param expression a boolean expression * @throws IllegalArgumentException if {@code expression} is false */ + @AssertMethod public static void checkArgument(boolean expression) { if (!expression) { throw new IllegalArgumentException(); @@ -131,6 +134,7 @@ public static void checkArgument(boolean expression) { * string using {@link String#valueOf(Object)} * @throws IllegalArgumentException if {@code expression} is false */ + @AssertMethod public static void checkArgument(boolean expression, Object errorMessage) { if (!expression) { throw new IllegalArgumentException(String.valueOf(errorMessage)); @@ -438,6 +442,7 @@ public static void checkArgument( * @param expression a boolean expression * @throws IllegalStateException if {@code expression} is false */ + @AssertMethod public static void checkState(boolean expression) { if (!expression) { throw new IllegalStateException(); @@ -453,6 +458,7 @@ public static void checkState(boolean expression) { * string using {@link String#valueOf(Object)} * @throws IllegalStateException if {@code expression} is false */ + @AssertMethod public static void checkState(boolean expression, Object errorMessage) { if (!expression) { throw new IllegalStateException(String.valueOf(errorMessage)); diff --git a/java/pom.xml b/java/pom.xml index 6b7192fd33e..62e63d41a91 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -48,6 +48,7 @@ 3.11.0 5.5.0 5.2.0 + 3.42.0 @@ -354,6 +355,7 @@ javax.annotation:javax.annotation-api:* org.apache.hadoop:hadoop-client-api + org.checkerframework:checker-qual @@ -606,6 +608,11 @@ pom import + + org.checkerframework + checker-qual + ${checker.framework.version} + com.google.flatbuffers flatbuffers-java