From 9471eed2978651079f35b5526365ca453503c9cb Mon Sep 17 00:00:00 2001 From: orozcosilverio Date: Mon, 22 Sep 2025 21:54:25 +0000 Subject: [PATCH 1/6] Refactoring QueueFile.java to avoid holding a leakable reference for a RandomAccessFile instance, instead a reference for the underlying file is kept. See https://github.com/firebase/firebase-android-sdk/issues/7341 for further context. --- .../internal/metadata/QueueFile.java | 302 +++++++++++------- .../internal/metadata/QueueFileLogStore.java | 2 - 2 files changed, 194 insertions(+), 110 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java index 78b976040ff..3559cb258d1 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java @@ -21,7 +21,6 @@ */ package com.google.firebase.crashlytics.internal.metadata; -import java.io.Closeable; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -36,14 +35,14 @@ * A reliable, efficient, file-based, FIFO queue. Additions and removals are O(1). All operations * are atomic. Writes are synchronous; data will be written to disk before an operation returns. The * underlying file is structured to survive process and even system crashes. If an I/O exception is - * thrown during a mutating change, the change is aborted. It is safe to continue to use a {@code - * QueueFile} instance after an exception. + * thrown during a mutating change, the change is aborted. It is safe to continue to use a + * {@code QueueFile} instance after an exception. * *

* *

All operations are synchronized. In a traditional queue, the remove operation returns an - * element. In this queue, {@link #peek} and {@link #remove} are used in conjunction. Use {@code - * peek} to retrieve the first element, and then {@code remove} to remove it after successful + * element. In this queue, {@link #peek} and {@link #remove} are used in conjunction. Use + * {@code peek} to retrieve the first element, and then {@code remove} to remove it after successful * processing. If the system crashes after {@code peek} and during processing, the element will * remain in the queue, to be processed when the system restarts. * @@ -57,13 +56,18 @@ * @author Bob Lee (bob@squareup.com) */ @SuppressWarnings("PMD") -class QueueFile implements Closeable { +class QueueFile { + private static final Logger LOGGER = Logger.getLogger(QueueFile.class.getName()); - /** Initial file size in bytes. */ + /** + * Initial file size in bytes. + */ private static final int INITIAL_LENGTH = 4096; // one file system block - /** Length of header in bytes. */ + /** + * Length of header in bytes. + */ static final int HEADER_LENGTH = 16; /** @@ -91,21 +95,31 @@ class QueueFile implements Closeable { * Data (Length bytes) * */ - private final RandomAccessFile raf; + private File rafFile; - /** Cached file length. Always a power of 2. */ + /** + * Cached file length. Always a power of 2. + */ int fileLength; - /** Number of elements. */ + /** + * Number of elements. + */ private int elementCount; - /** Pointer to first (or eldest) element. */ + /** + * Pointer to first (or eldest) element. + */ private Element first; - /** Pointer to last (or newest) element. */ + /** + * Pointer to last (or newest) element. + */ private Element last; - /** In-memory buffer. Big enough to hold the header. */ + /** + * In-memory buffer. Big enough to hold the header. + */ private final byte[] buffer = new byte[16]; /** @@ -116,19 +130,12 @@ public QueueFile(File file) throws IOException { if (!file.exists()) { initialize(file); } - raf = open(file); - readHeader(); - } - - /** For testing. */ - QueueFile(RandomAccessFile raf) throws IOException { - this.raf = raf; readHeader(); } /** - * Stores int in buffer. The behavior is equivalent to calling {@link - * java.io.RandomAccessFile#writeInt}. + * Stores int in buffer. The behavior is equivalent to calling + * {@link java.io.RandomAccessFile#writeInt}. */ private static void writeInt(byte[] buffer, int offset, int value) { buffer[offset] = (byte) (value >> 24); @@ -138,8 +145,8 @@ private static void writeInt(byte[] buffer, int offset, int value) { } /** - * Stores int values in buffer. The behavior is equivalent to calling {@link - * java.io.RandomAccessFile#writeInt} for each value. + * Stores int values in buffer. The behavior is equivalent to calling + * {@link java.io.RandomAccessFile#writeInt} for each value. */ private static void writeInts(byte[] buffer, int... values) { int offset = 0; @@ -149,7 +156,9 @@ private static void writeInts(byte[] buffer, int... values) { } } - /** Reads an int from a byte[]. */ + /** + * Reads an int from a byte[]. + */ private static int readInt(byte[] buffer, int offset) { return ((buffer[offset] & 0xff) << 24) + ((buffer[offset + 1] & 0xff) << 16) @@ -157,20 +166,25 @@ private static int readInt(byte[] buffer, int offset) { + (buffer[offset + 3] & 0xff); } - /** Reads the header. */ + /** + * Reads the header. + */ private void readHeader() throws IOException { - raf.seek(0); - raf.readFully(buffer); - fileLength = readInt(buffer, 0); - if (fileLength > raf.length()) { - throw new IOException( - "File is truncated. Expected length: " + fileLength + ", Actual length: " + raf.length()); - } - elementCount = readInt(buffer, 4); - int firstOffset = readInt(buffer, 8); - int lastOffset = readInt(buffer, 12); - first = readElement(firstOffset); - last = readElement(lastOffset); + openAndExecute(raf -> { + raf.seek(0); + raf.readFully(buffer); + fileLength = readInt(buffer, 0); + if (fileLength > raf.length()) { + throw new IOException( + "File is truncated. Expected length: " + fileLength + ", Actual length: " + + raf.length()); + } + elementCount = readInt(buffer, 4); + int firstOffset = readInt(buffer, 8); + int lastOffset = readInt(buffer, 12); + first = readElement(firstOffset); + last = readElement(lastOffset); + }); } /** @@ -182,46 +196,65 @@ private void readHeader() throws IOException { private void writeHeader(int fileLength, int elementCount, int firstPosition, int lastPosition) throws IOException { writeInts(buffer, fileLength, elementCount, firstPosition, lastPosition); - raf.seek(0); - raf.write(buffer); + openAndExecute(raf -> { + raf.seek(0); + raf.write(buffer); + }); } - /** Returns the Element for the given offset. */ + /** + * Returns the Element for the given offset. + */ private Element readElement(int position) throws IOException { if (position == 0) { return Element.NULL; } - raf.seek(position); - return new Element(position, raf.readInt()); + try (RandomAccessFile raf = open(rafFile)) { + raf.seek(position); + return new Element(position, raf.readInt()); + } } - /** Atomically initializes a new file. */ - private static void initialize(File file) throws IOException { + /** + * Atomically initializes a new file. + */ + private void initialize(File file) throws IOException { // Use a temp file so we don't leave a partially-initialized file. File tempFile = new File(file.getPath() + ".tmp"); - RandomAccessFile raf = open(tempFile); - try { + openAndExecute(raf -> { raf.setLength(INITIAL_LENGTH); raf.seek(0); byte[] headerBuffer = new byte[16]; writeInts(headerBuffer, INITIAL_LENGTH, 0, 0, 0); raf.write(headerBuffer); - } finally { - raf.close(); - } + }); // A rename is atomic. if (!tempFile.renameTo(file)) { throw new IOException("Rename failed!"); } + this.rafFile = tempFile; } - /** Opens a random access file that writes synchronously. */ + /** + * Opens a random access file that writes synchronously. + */ private static RandomAccessFile open(File file) throws FileNotFoundException { return new RandomAccessFile(file, "rwd"); } - /** Wraps the position if it exceeds the end of the file. */ + /** + * Opens a random access file that writes synchronously and executes the provided callback. + */ + private void openAndExecute(RandomAccessFileCallable rafCallable) throws IOException { + try (RandomAccessFile raf = open(rafFile)) { + rafCallable.run(raf); + } + } + + /** + * Wraps the position if it exceeds the end of the file. + */ private int wrapPosition(int position) { return position < fileLength ? position : HEADER_LENGTH + position - fileLength; } @@ -235,19 +268,21 @@ private int wrapPosition(int position) { * @param count # of bytes to write */ private void ringWrite(int position, byte[] buffer, int offset, int count) throws IOException { - position = wrapPosition(position); - if (position + count <= fileLength) { - raf.seek(position); - raf.write(buffer, offset, count); - } else { - // The write overlaps the EOF. - // # of bytes to write before the EOF. - int beforeEof = fileLength - position; - raf.seek(position); - raf.write(buffer, offset, beforeEof); - raf.seek(HEADER_LENGTH); - raf.write(buffer, offset + beforeEof, count - beforeEof); - } + final int wrappedPosition = wrapPosition(position); + openAndExecute(raf -> { + if (wrappedPosition + count <= fileLength) { + raf.seek(wrappedPosition); + raf.write(buffer, offset, count); + } else { + // The write overlaps the EOF. + // # of bytes to write before the EOF. + int beforeEof = fileLength - wrappedPosition; + raf.seek(wrappedPosition); + raf.write(buffer, offset, beforeEof); + raf.seek(HEADER_LENGTH); + raf.write(buffer, offset + beforeEof, count - beforeEof); + } + }); } /** @@ -258,19 +293,21 @@ private void ringWrite(int position, byte[] buffer, int offset, int count) throw * @param count # of bytes to read */ private void ringRead(int position, byte[] buffer, int offset, int count) throws IOException { - position = wrapPosition(position); - if (position + count <= fileLength) { - raf.seek(position); - raf.readFully(buffer, offset, count); - } else { - // The read overlaps the EOF. - // # of bytes to read before the EOF. - int beforeEof = fileLength - position; - raf.seek(position); - raf.readFully(buffer, offset, beforeEof); - raf.seek(HEADER_LENGTH); - raf.readFully(buffer, offset + beforeEof, count - beforeEof); - } + final int wrappedPosition = wrapPosition(position); + openAndExecute(raf -> { + if (wrappedPosition + count <= fileLength) { + raf.seek(wrappedPosition); + raf.readFully(buffer, offset, count); + } else { + // The read overlaps the EOF. + // # of bytes to read before the EOF. + int beforeEof = fileLength - wrappedPosition; + raf.seek(wrappedPosition); + raf.readFully(buffer, offset, beforeEof); + raf.seek(HEADER_LENGTH); + raf.readFully(buffer, offset + beforeEof, count - beforeEof); + } + }); } /** @@ -288,8 +325,8 @@ public void add(byte[] data) throws IOException { * @param data to copy bytes from * @param offset to start from in buffer * @param count number of bytes to copy - * @throws IndexOutOfBoundsException if {@code offset < 0} or {@code count < 0}, or if {@code - * offset + count} is bigger than the length of {@code buffer}. + * @throws IndexOutOfBoundsException if {@code offset < 0} or {@code count < 0}, or if + * {@code offset + count} is bigger than the length of {@code buffer}. */ public synchronized void add(byte[] data, int offset, int count) throws IOException { nonNull(data, "buffer"); @@ -324,7 +361,9 @@ public synchronized void add(byte[] data, int offset, int count) throws IOExcept } } - /** Returns the number of used bytes. */ + /** + * Returns the number of used bytes. + */ public int usedBytes() { if (elementCount == 0) { return HEADER_LENGTH; @@ -346,12 +385,16 @@ public int usedBytes() { } } - /** Returns number of unused bytes. */ + /** + * Returns number of unused bytes. + */ private int remainingBytes() { return fileLength - usedBytes(); } - /** Returns true if this queue contains no entries. */ + /** + * Returns true if this queue contains no entries. + */ public synchronized boolean isEmpty() { return elementCount == 0; } @@ -385,7 +428,10 @@ private void expandIfNecessary(int dataLength) throws IOException { // If the buffer is split, we need to make it contiguous if (endOfLastElement < first.position) { - FileChannel channel = raf.getChannel(); + FileChannel channel; + try (RandomAccessFile raf = open(rafFile)) { + channel = raf.getChannel(); + } channel.position(fileLength); // destination position int count = endOfLastElement - Element.HEADER_LENGTH; if (channel.transferTo(HEADER_LENGTH, count, channel) != count) { @@ -405,14 +451,20 @@ private void expandIfNecessary(int dataLength) throws IOException { fileLength = newLength; } - /** Sets the length of the file. */ + /** + * Sets the length of the file. + */ private void setLength(int newLength) throws IOException { - // Set new file length (considered metadata) and sync it to storage. - raf.setLength(newLength); - raf.getChannel().force(true); + openAndExecute(raf -> { + // Set new file length (considered metadata) and sync it to storage. + raf.setLength(newLength); + raf.getChannel().force(true); + }); } - /** Reads the eldest element. Returns null if the queue is empty. */ + /** + * Reads the eldest element. Returns null if the queue is empty. + */ public synchronized byte[] peek() throws IOException { if (isEmpty()) { return null; @@ -423,7 +475,9 @@ public synchronized byte[] peek() throws IOException { return data; } - /** Invokes reader with the eldest element, if an element is available. */ + /** + * Invokes reader with the eldest element, if an element is available. + */ public synchronized void peek(ElementReader reader) throws IOException { if (elementCount > 0) { reader.read(new ElementInputStream(first), first.length); @@ -455,8 +509,11 @@ private static T nonNull(T t, String name) { return t; } - /** Reads a single element. */ + /** + * Reads a single element. + */ private final class ElementInputStream extends InputStream { + private int position; private int remaining; @@ -489,15 +546,20 @@ public int read() throws IOException { if (remaining == 0) { return -1; } - raf.seek(position); - int b = raf.read(); + int readByte; + try (RandomAccessFile raf = open(rafFile)) { + raf.seek(position); + readByte = raf.read(); + } position = wrapPosition(position + 1); remaining--; - return b; + return readByte; } } - /** Returns the number of elements in this queue. */ + /** + * Returns the number of elements in this queue. + */ public synchronized int size() { return elementCount; } @@ -524,21 +586,20 @@ public synchronized void remove() throws IOException { } } - /** Clears this queue. Truncates the file to the initial size. */ + /** + * Clears this queue. Truncates the file to the initial size. + */ public synchronized void clear() throws IOException { writeHeader(INITIAL_LENGTH, 0, 0, 0); elementCount = 0; first = Element.NULL; last = Element.NULL; - if (fileLength > INITIAL_LENGTH) setLength(INITIAL_LENGTH); + if (fileLength > INITIAL_LENGTH) { + setLength(INITIAL_LENGTH); + } fileLength = INITIAL_LENGTH; } - /** Closes the underlying file. */ - public synchronized void close() throws IOException { - raf.close(); - } - /** * Returns whether the current used data size, plus the data size provided, plus element header * size will stay within the max size provided @@ -578,19 +639,29 @@ public void read(InputStream in, int length) throws IOException { return builder.toString(); } - /** A pointer to an element. */ + /** + * A pointer to an element. + */ static class Element { - /** Length of element header in bytes. */ + /** + * Length of element header in bytes. + */ static final int HEADER_LENGTH = 4; - /** Null element. */ + /** + * Null element. + */ static final Element NULL = new Element(0, 0); - /** Position in file. */ + /** + * Position in file. + */ final int position; - /** The length of the data. */ + /** + * The length of the data. + */ final int length; /** @@ -621,6 +692,7 @@ public String toString() { * byte[]. */ public interface ElementReader { + /** * Called once per element. * @@ -631,4 +703,18 @@ public interface ElementReader { */ void read(InputStream in, int length) throws IOException; } + + /** + * SAM interface util for reusable interactions with RandomAccessFile. + */ + private interface RandomAccessFileCallable { + + /** + * Callback with scope-specific actions to be executed along RandomAccessFile open & close + * actions. + * + * @param file RandomAccessFile provided by {@link #open(File)} + */ + void run(RandomAccessFile file) throws IOException; + } } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFileLogStore.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFileLogStore.java index 23f7b5bae29..96cf2a37b1c 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFileLogStore.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFileLogStore.java @@ -15,7 +15,6 @@ package com.google.firebase.crashlytics.internal.metadata; import com.google.firebase.crashlytics.internal.Logger; -import com.google.firebase.crashlytics.internal.common.CommonUtils; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -112,7 +111,6 @@ public void read(InputStream in, int length) throws IOException { @Override public void closeLogFile() { - CommonUtils.closeOrLog(logFile, "There was a problem closing the Crashlytics log file."); logFile = null; } From 3a091bd2ebba894643e65b71cf5b394b7131c0b3 Mon Sep 17 00:00:00 2001 From: orozcosilverio Date: Wed, 24 Sep 2025 22:15:13 +0000 Subject: [PATCH 2/6] Addressing Code Review (possible NPE and mistaken file at initialization). --- .../firebase/crashlytics/internal/metadata/QueueFile.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java index 3559cb258d1..ab487599f4a 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java @@ -95,7 +95,7 @@ class QueueFile { * Data (Length bytes) * */ - private File rafFile; + private final File rafFile; /** * Cached file length. Always a power of 2. @@ -127,6 +127,7 @@ class QueueFile { * access a given file at a time. */ public QueueFile(File file) throws IOException { + this.rafFile = file; if (!file.exists()) { initialize(file); } @@ -221,19 +222,18 @@ private Element readElement(int position) throws IOException { private void initialize(File file) throws IOException { // Use a temp file so we don't leave a partially-initialized file. File tempFile = new File(file.getPath() + ".tmp"); - openAndExecute(raf -> { + try (RandomAccessFile raf = open(tempFile)) { raf.setLength(INITIAL_LENGTH); raf.seek(0); byte[] headerBuffer = new byte[16]; writeInts(headerBuffer, INITIAL_LENGTH, 0, 0, 0); raf.write(headerBuffer); - }); + } // A rename is atomic. if (!tempFile.renameTo(file)) { throw new IOException("Rename failed!"); } - this.rafFile = tempFile; } /** From 5056a8fbf642b1dbe041fae3d2dc77b1bd7446e9 Mon Sep 17 00:00:00 2001 From: orozcosilverio Date: Wed, 24 Sep 2025 22:32:53 +0000 Subject: [PATCH 3/6] NIT on Method Signature. --- .../firebase/crashlytics/internal/metadata/QueueFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java index ab487599f4a..0977f4b3335 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java @@ -219,7 +219,7 @@ private Element readElement(int position) throws IOException { /** * Atomically initializes a new file. */ - private void initialize(File file) throws IOException { + private static void initialize(File file) throws IOException { // Use a temp file so we don't leave a partially-initialized file. File tempFile = new File(file.getPath() + ".tmp"); try (RandomAccessFile raf = open(tempFile)) { From a820d3182c2828ee0539b793c845dfefdcf21475 Mon Sep 17 00:00:00 2001 From: orozcosilverio Date: Thu, 25 Sep 2025 00:00:36 +0000 Subject: [PATCH 4/6] Aim for the right RAF method in order to handle FileChannel instance. --- .../internal/metadata/QueueFile.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java index 0977f4b3335..6e862c9d236 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java @@ -428,15 +428,14 @@ private void expandIfNecessary(int dataLength) throws IOException { // If the buffer is split, we need to make it contiguous if (endOfLastElement < first.position) { - FileChannel channel; - try (RandomAccessFile raf = open(rafFile)) { - channel = raf.getChannel(); - } - channel.position(fileLength); // destination position - int count = endOfLastElement - Element.HEADER_LENGTH; - if (channel.transferTo(HEADER_LENGTH, count, channel) != count) { - throw new AssertionError("Copied insufficient number of bytes!"); - } + openAndExecute(raf -> { + FileChannel channel = raf.getChannel(); + channel.position(fileLength); // destination position + int count = endOfLastElement - Element.HEADER_LENGTH; + if (channel.transferTo(HEADER_LENGTH, count, channel) != count) { + throw new AssertionError("Copied insufficient number of bytes!"); + } + }); } // Commit the expansion. From 98011990af7f75b52b9d78c85bcf63ca73f42a49 Mon Sep 17 00:00:00 2001 From: orozcosilverio Date: Thu, 25 Sep 2025 01:35:33 +0000 Subject: [PATCH 5/6] Optimizing the use of RandomAccessFile with the help of local variables. --- .../internal/metadata/QueueFile.java | 214 +++++++++--------- 1 file changed, 107 insertions(+), 107 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java index 6e862c9d236..25da849fe3d 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java @@ -183,8 +183,8 @@ private void readHeader() throws IOException { elementCount = readInt(buffer, 4); int firstOffset = readInt(buffer, 8); int lastOffset = readInt(buffer, 12); - first = readElement(firstOffset); - last = readElement(lastOffset); + first = readElement(firstOffset, raf); + last = readElement(lastOffset, raf); }); } @@ -194,26 +194,23 @@ private void readHeader() throws IOException { * update the class member variables *after* this call succeeds. Assumes segment writes are atomic * in the underlying file system. */ - private void writeHeader(int fileLength, int elementCount, int firstPosition, int lastPosition) + private void writeHeader(int fileLength, int elementCount, int firstPosition, int lastPosition, + RandomAccessFile raf) throws IOException { writeInts(buffer, fileLength, elementCount, firstPosition, lastPosition); - openAndExecute(raf -> { - raf.seek(0); - raf.write(buffer); - }); + raf.seek(0); + raf.write(buffer); } /** * Returns the Element for the given offset. */ - private Element readElement(int position) throws IOException { + private Element readElement(int position, RandomAccessFile raf) throws IOException { if (position == 0) { return Element.NULL; } - try (RandomAccessFile raf = open(rafFile)) { - raf.seek(position); - return new Element(position, raf.readInt()); - } + raf.seek(position); + return new Element(position, raf.readInt()); } /** @@ -266,23 +263,23 @@ private int wrapPosition(int position) { * @param position in file to write to * @param buffer to write from * @param count # of bytes to write + * @param raf scoped underlying file */ - private void ringWrite(int position, byte[] buffer, int offset, int count) throws IOException { - final int wrappedPosition = wrapPosition(position); - openAndExecute(raf -> { - if (wrappedPosition + count <= fileLength) { - raf.seek(wrappedPosition); - raf.write(buffer, offset, count); - } else { - // The write overlaps the EOF. - // # of bytes to write before the EOF. - int beforeEof = fileLength - wrappedPosition; - raf.seek(wrappedPosition); - raf.write(buffer, offset, beforeEof); - raf.seek(HEADER_LENGTH); - raf.write(buffer, offset + beforeEof, count - beforeEof); - } - }); + private void ringWrite(int position, byte[] buffer, int offset, int count, RandomAccessFile raf) + throws IOException { + position = wrapPosition(position); + if (position + count <= fileLength) { + raf.seek(position); + raf.write(buffer, offset, count); + } else { + // The write overlaps the EOF. + // # of bytes to write before the EOF. + int beforeEof = fileLength - position; + raf.seek(position); + raf.write(buffer, offset, beforeEof); + raf.seek(HEADER_LENGTH); + raf.write(buffer, offset + beforeEof, count - beforeEof); + } } /** @@ -291,23 +288,23 @@ private void ringWrite(int position, byte[] buffer, int offset, int count) throw * @param position in file to read from * @param buffer to read into * @param count # of bytes to read + * @param raf scoped underlying file */ - private void ringRead(int position, byte[] buffer, int offset, int count) throws IOException { - final int wrappedPosition = wrapPosition(position); - openAndExecute(raf -> { - if (wrappedPosition + count <= fileLength) { - raf.seek(wrappedPosition); - raf.readFully(buffer, offset, count); - } else { - // The read overlaps the EOF. - // # of bytes to read before the EOF. - int beforeEof = fileLength - wrappedPosition; - raf.seek(wrappedPosition); - raf.readFully(buffer, offset, beforeEof); - raf.seek(HEADER_LENGTH); - raf.readFully(buffer, offset + beforeEof, count - beforeEof); - } - }); + private void ringRead(int position, byte[] buffer, int offset, int count, RandomAccessFile raf) + throws IOException { + position = wrapPosition(position); + if (position + count <= fileLength) { + raf.seek(position); + raf.readFully(buffer, offset, count); + } else { + // The read overlaps the EOF. + // # of bytes to read before the EOF. + int beforeEof = fileLength - position; + raf.seek(position); + raf.readFully(buffer, offset, beforeEof); + raf.seek(HEADER_LENGTH); + raf.readFully(buffer, offset + beforeEof, count - beforeEof); + } } /** @@ -334,31 +331,32 @@ public synchronized void add(byte[] data, int offset, int count) throws IOExcept throw new IndexOutOfBoundsException(); } - expandIfNecessary(count); - - // Insert a new element after the current last element. - boolean wasEmpty = isEmpty(); - int position = - wasEmpty - ? HEADER_LENGTH - : wrapPosition(last.position + Element.HEADER_LENGTH + last.length); - Element newLast = new Element(position, count); - - // Write length. - writeInt(buffer, 0, count); - ringWrite(newLast.position, buffer, 0, Element.HEADER_LENGTH); - - // Write data. - ringWrite(newLast.position + Element.HEADER_LENGTH, data, offset, count); - - // Commit the addition. If wasEmpty, first == last. - int firstPosition = wasEmpty ? newLast.position : first.position; - writeHeader(fileLength, elementCount + 1, firstPosition, newLast.position); - last = newLast; - elementCount++; - if (wasEmpty) { - first = last; // first element - } + openAndExecute(raf -> { + expandIfNecessary(count, raf); + + // Insert a new element after the current last element. + boolean wasEmpty = isEmpty(); + int position = + wasEmpty + ? HEADER_LENGTH + : wrapPosition(last.position + Element.HEADER_LENGTH + last.length); + Element newLast = new Element(position, count); + + // Write length. + writeInt(buffer, 0, count); + ringWrite(newLast.position, buffer, 0, Element.HEADER_LENGTH, raf); + + // Write data. + ringWrite(newLast.position + Element.HEADER_LENGTH, data, offset, count, raf); + // Commit the addition. If wasEmpty, first == last. + int firstPosition = wasEmpty ? newLast.position : first.position; + writeHeader(fileLength, elementCount + 1, firstPosition, newLast.position, raf); + last = newLast; + elementCount++; + if (wasEmpty) { + first = last; // first element + } + }); } /** @@ -404,7 +402,7 @@ public synchronized boolean isEmpty() { * * @param dataLength length of data being added */ - private void expandIfNecessary(int dataLength) throws IOException { + private void expandIfNecessary(int dataLength, RandomAccessFile raf) throws IOException { int elementLength = Element.HEADER_LENGTH + dataLength; int remainingBytes = remainingBytes(); if (remainingBytes >= elementLength) { @@ -428,23 +426,21 @@ private void expandIfNecessary(int dataLength) throws IOException { // If the buffer is split, we need to make it contiguous if (endOfLastElement < first.position) { - openAndExecute(raf -> { - FileChannel channel = raf.getChannel(); - channel.position(fileLength); // destination position - int count = endOfLastElement - Element.HEADER_LENGTH; - if (channel.transferTo(HEADER_LENGTH, count, channel) != count) { - throw new AssertionError("Copied insufficient number of bytes!"); - } - }); + FileChannel channel = raf.getChannel(); + channel.position(fileLength); // destination position + int count = endOfLastElement - Element.HEADER_LENGTH; + if (channel.transferTo(HEADER_LENGTH, count, channel) != count) { + throw new AssertionError("Copied insufficient number of bytes!"); + } } // Commit the expansion. if (last.position < first.position) { int newLastPosition = fileLength + last.position - HEADER_LENGTH; - writeHeader(newLength, elementCount, first.position, newLastPosition); + writeHeader(newLength, elementCount, first.position, newLastPosition, raf); last = new Element(newLastPosition, last.length); } else { - writeHeader(newLength, elementCount, first.position, last.position); + writeHeader(newLength, elementCount, first.position, last.position, raf); } fileLength = newLength; @@ -470,7 +466,7 @@ public synchronized byte[] peek() throws IOException { } int length = first.length; byte[] data = new byte[length]; - ringRead(first.position + Element.HEADER_LENGTH, data, 0, length); + openAndExecute(raf -> ringRead(first.position + Element.HEADER_LENGTH, data, 0, length, raf)); return data; } @@ -479,7 +475,7 @@ public synchronized byte[] peek() throws IOException { */ public synchronized void peek(ElementReader reader) throws IOException { if (elementCount > 0) { - reader.read(new ElementInputStream(first), first.length); + openAndExecute(raf -> reader.read(new ElementInputStream(first, raf), first.length)); } } @@ -488,12 +484,14 @@ public synchronized void peek(ElementReader reader) throws IOException { * added. */ public synchronized void forEach(ElementReader reader) throws IOException { - int position = first.position; - for (int i = 0; i < elementCount; i++) { - Element current = readElement(position); - reader.read(new ElementInputStream(current), current.length); - position = wrapPosition(current.position + Element.HEADER_LENGTH + current.length); - } + openAndExecute(raf -> { + int position = first.position; + for (int i = 0; i < elementCount; i++) { + Element current = readElement(position, raf); + reader.read(new ElementInputStream(current, raf), current.length); + position = wrapPosition(current.position + Element.HEADER_LENGTH + current.length); + } + }); } /** @@ -515,10 +513,12 @@ private final class ElementInputStream extends InputStream { private int position; private int remaining; + private final RandomAccessFile raf; - private ElementInputStream(Element element) { + private ElementInputStream(Element element, RandomAccessFile raf) { position = wrapPosition(element.position + Element.HEADER_LENGTH); remaining = element.length; + this.raf = raf; } @Override @@ -531,7 +531,7 @@ public int read(byte[] buffer, int offset, int length) throws IOException { if (length > remaining) { length = remaining; } - ringRead(position, buffer, offset, length); + ringRead(position, buffer, offset, length, raf); position = wrapPosition(position + length); remaining -= length; return length; @@ -546,10 +546,8 @@ public int read() throws IOException { return -1; } int readByte; - try (RandomAccessFile raf = open(rafFile)) { - raf.seek(position); - readByte = raf.read(); - } + raf.seek(position); + readByte = raf.read(); position = wrapPosition(position + 1); remaining--; return readByte; @@ -572,24 +570,26 @@ public synchronized void remove() throws IOException { if (isEmpty()) { throw new NoSuchElementException(); } - if (elementCount == 1) { - clear(); - } else { - // assert elementCount > 1 - int newFirstPosition = wrapPosition(first.position + Element.HEADER_LENGTH + first.length); - ringRead(newFirstPosition, buffer, 0, Element.HEADER_LENGTH); - int length = readInt(buffer, 0); - writeHeader(fileLength, elementCount - 1, newFirstPosition, last.position); - elementCount--; - first = new Element(newFirstPosition, length); - } + openAndExecute(raf -> { + if (elementCount == 1) { + clear(raf); + } else { + // assert elementCount > 1 + int newFirstPosition = wrapPosition(first.position + Element.HEADER_LENGTH + first.length); + ringRead(newFirstPosition, buffer, 0, Element.HEADER_LENGTH, raf); + int length = readInt(buffer, 0); + writeHeader(fileLength, elementCount - 1, newFirstPosition, last.position, raf); + elementCount--; + first = new Element(newFirstPosition, length); + } + }); } /** * Clears this queue. Truncates the file to the initial size. */ - public synchronized void clear() throws IOException { - writeHeader(INITIAL_LENGTH, 0, 0, 0); + public synchronized void clear(RandomAccessFile raf) throws IOException { + writeHeader(INITIAL_LENGTH, 0, 0, 0, raf); elementCount = 0; first = Element.NULL; last = Element.NULL; From 19697587235decd7aac7bec03a6dd369c7551541 Mon Sep 17 00:00:00 2001 From: orozcosilverio Date: Thu, 25 Sep 2025 01:42:18 +0000 Subject: [PATCH 6/6] Refining the use of RandomAccessFile with the help of local variables. --- .../crashlytics/internal/metadata/QueueFile.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java index 25da849fe3d..9c72fe36474 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/metadata/QueueFile.java @@ -419,7 +419,7 @@ private void expandIfNecessary(int dataLength, RandomAccessFile raf) throws IOEx previousLength = newLength; } while (remainingBytes < elementLength); - setLength(newLength); + setLength(newLength, raf); // Calculate the position of the tail end of the data in the ring buffer int endOfLastElement = wrapPosition(last.position + Element.HEADER_LENGTH + last.length); @@ -449,12 +449,10 @@ private void expandIfNecessary(int dataLength, RandomAccessFile raf) throws IOEx /** * Sets the length of the file. */ - private void setLength(int newLength) throws IOException { - openAndExecute(raf -> { - // Set new file length (considered metadata) and sync it to storage. - raf.setLength(newLength); - raf.getChannel().force(true); - }); + private void setLength(int newLength, RandomAccessFile raf) throws IOException { + // Set new file length (considered metadata) and sync it to storage. + raf.setLength(newLength); + raf.getChannel().force(true); } /** @@ -588,13 +586,13 @@ public synchronized void remove() throws IOException { /** * Clears this queue. Truncates the file to the initial size. */ - public synchronized void clear(RandomAccessFile raf) throws IOException { + private synchronized void clear(RandomAccessFile raf) throws IOException { writeHeader(INITIAL_LENGTH, 0, 0, 0, raf); elementCount = 0; first = Element.NULL; last = Element.NULL; if (fileLength > INITIAL_LENGTH) { - setLength(INITIAL_LENGTH); + setLength(INITIAL_LENGTH, raf); } fileLength = INITIAL_LENGTH; }