Reduce internal boilerplate
Make the internal scratch byte and char buffers auto-closeable
diff --git a/src/main/java/org/apache/commons/io/CopyUtils.java b/src/main/java/org/apache/commons/io/CopyUtils.java
index fdb137d..b3243a3 100644
--- a/src/main/java/org/apache/commons/io/CopyUtils.java
+++ b/src/main/java/org/apache/commons/io/CopyUtils.java
@@ -29,6 +29,8 @@
import java.io.Writer;
import java.nio.charset.Charset;
+import org.apache.commons.io.IOUtils.ScratchChars;
+
/**
* This class provides static utility methods for buffered
* copying between sources ({@link InputStream}, {@link Reader},
@@ -278,8 +280,8 @@ public static int copy(
final Reader input,
final Writer output)
throws IOException {
- final char[] buffer = IOUtils.ScratchBufferHolder.getScratchCharArray();
- try {
+ try (ScratchChars scratch = IOUtils.ScratchChars.get()) {
+ final char[] buffer = scratch.array();
int count = 0;
int n;
while (EOF != (n = input.read(buffer))) {
@@ -287,8 +289,6 @@ public static int copy(
count += n;
}
return count;
- } finally {
- IOUtils.ScratchBufferHolder.releaseScratchCharArray(buffer);
}
}
diff --git a/src/main/java/org/apache/commons/io/IOUtils.java b/src/main/java/org/apache/commons/io/IOUtils.java
index fb6bc6c..5d3d7cc 100644
--- a/src/main/java/org/apache/commons/io/IOUtils.java
+++ b/src/main/java/org/apache/commons/io/IOUtils.java
@@ -133,7 +133,7 @@ public class IOUtils {
// or return one of them.
/**
- * Holder for per-thread internal scratch buffers.
+ * Holder for per-thread internal scratch buffer.
*
* <p>Buffers are created lazily and reused within the same thread to reduce allocation overhead. In the rare case of reentrant access, a temporary buffer
* is allocated to avoid data corruption.</p>
@@ -141,80 +141,120 @@ public class IOUtils {
* <p>Typical usage:</p>
*
* <pre>{@code
- * final byte[] buffer = ScratchBufferHolder.getScratchByteArray();
- * try {
+ * try (ScratchBytes scratch = ScratchBytes.get()) {
* // use the buffer
- * } finally {
- * ScratchBufferHolder.releaseScratchByteArray(buffer);
+ * byte[] bytes = scratch.array();
+ * // ...
* }
* }</pre>
*/
- static final class ScratchBufferHolder {
+ static final class ScratchBytes implements AutoCloseable {
/**
- * Holder for internal byte array buffer.
+ * Wraps an internal byte array.
+ *
+ * [0] boolean in use.
+ * [1] byte[] buffer.
*/
- private static final ThreadLocal<Object[]> SCRATCH_BYTE_BUFFER_HOLDER = ThreadLocal.withInitial(() -> new Object[] { false, byteArray() });
-
- /**
- * Holder for internal char array buffer.
- */
- private static final ThreadLocal<Object[]> SCRATCH_CHAR_BUFFER_HOLDER = ThreadLocal.withInitial(() -> new Object[] { false, charArray() });
-
+ private static final ThreadLocal<Object[]> LOCAL = ThreadLocal.withInitial(() -> new Object[] { false, new ScratchBytes(byteArray()) });
/**
* Gets the internal byte array buffer.
*
* @return the internal byte array buffer.
*/
- static byte[] getScratchByteArray() {
- final Object[] holder = SCRATCH_BYTE_BUFFER_HOLDER.get();
+ static ScratchBytes get() {
+ final Object[] holder = LOCAL.get();
// If already in use, return a new array
if ((boolean) holder[0]) {
- return byteArray();
+ return new ScratchBytes(byteArray());
}
holder[0] = true;
- return (byte[]) holder[1];
+ return (ScratchBytes) holder[1];
+ }
+
+ private final byte[] buffer;
+
+ private ScratchBytes(final byte[] buffer) {
+ this.buffer = buffer;
+ }
+
+ byte[] array() {
+ return buffer;
}
/**
- * Gets the char array buffer.
- *
- * @return the char array buffer.
+ * If the buffer is the internal array, clear and release it for reuse.
*/
- static char[] getScratchCharArray() {
- final Object[] holder = SCRATCH_CHAR_BUFFER_HOLDER.get();
- // If already in use, return a new array
- if ((boolean) holder[0]) {
- return charArray();
- }
- holder[0] = true;
- return (char[]) holder[1];
- }
-
-
- /**
- * If the argument is the internal byte array, release it for reuse.
- *
- * @param array the byte array to release.
- */
- static void releaseScratchByteArray(final byte[] array) {
- final Object[] holder = SCRATCH_BYTE_BUFFER_HOLDER.get();
- if (array == holder[1]) {
- Arrays.fill(array, (byte) 0);
+ @Override
+ public void close() {
+ final Object[] holder = LOCAL.get();
+ if (buffer == ((ScratchBytes) holder[1]).buffer) {
+ Arrays.fill(buffer, (byte) 0);
holder[0] = false;
}
}
+ }
+
+ /**
+ * Holder for per-thread internal scratch buffer.
+ *
+ * <p>Buffers are created lazily and reused within the same thread to reduce allocation overhead. In the rare case of reentrant access, a temporary buffer
+ * is allocated to avoid data corruption.</p>
+ *
+ * <p>Typical usage:</p>
+ *
+ * <pre>{@code
+ * try (ScratchChars scratch = ScratchChars.get()) {
+ * // use the buffer
+ * char[] bytes = scratch.array();
+ * // ...
+ * }
+ * }</pre>
+ */
+ static final class ScratchChars implements AutoCloseable {
/**
- * If the argument is the internal char array, release it for reuse.
+ * Wraps an internal char array.
*
- * @param array the char array to release.
+ * [0] boolean in use.
+ * [1] char[] buffer.
*/
- static void releaseScratchCharArray(final char[] array) {
- final Object[] holder = SCRATCH_CHAR_BUFFER_HOLDER.get();
- if (array == holder[1]) {
- Arrays.fill(array, (char) 0);
+ private static final ThreadLocal<Object[]> LOCAL = ThreadLocal.withInitial(() -> new Object[] { false, new ScratchChars(charArray()) });
+
+ /**
+ * Gets the internal char array buffer.
+ *
+ * @return the internal char array buffer.
+ */
+ static ScratchChars get() {
+ final Object[] holder = LOCAL.get();
+ // If already in use, return a new array
+ if ((boolean) holder[0]) {
+ return new ScratchChars(charArray());
+ }
+ holder[0] = true;
+ return (ScratchChars) holder[1];
+ }
+
+ private final char[] buffer;
+
+ private ScratchChars(final char[] buffer) {
+ this.buffer = buffer;
+ }
+
+ char[] array() {
+ return buffer;
+ }
+
+ /**
+ * If the buffer is the internal array, clear and release it for reuse.
+ */
+ @Override
+ public void close() {
+ final Object[] holder = LOCAL.get();
+ if (buffer == ((ScratchChars) holder[1]).buffer) {
+ Arrays.fill(buffer, (char) 0);
holder[0] = false;
}
}
@@ -660,8 +700,8 @@ static void checkFromToIndex(final int fromIndex, final int toIndex, final int l
* @see IO#clear()
*/
static void clear() {
- ScratchBufferHolder.SCRATCH_BYTE_BUFFER_HOLDER.remove();
- ScratchBufferHolder.SCRATCH_CHAR_BUFFER_HOLDER.remove();
+ ScratchBytes.LOCAL.remove();
+ ScratchChars.LOCAL.remove();
}
/**
@@ -1205,14 +1245,14 @@ public static boolean contentEquals(final Reader input1, final Reader input2) th
}
// reuse one
- final char[] array1 = ScratchBufferHolder.getScratchCharArray();
- // but allocate another
- final char[] array2 = charArray();
- int pos1;
- int pos2;
- int count1;
- int count2;
- try {
+ try (ScratchChars scratch = IOUtils.ScratchChars.get()) {
+ final char[] array1 = scratch.array();
+ // but allocate another
+ final char[] array2 = charArray();
+ int pos1;
+ int pos2;
+ int count1;
+ int count2;
while (true) {
pos1 = 0;
pos2 = 0;
@@ -1240,8 +1280,6 @@ public static boolean contentEquals(final Reader input1, final Reader input2) th
}
}
}
- } finally {
- ScratchBufferHolder.releaseScratchCharArray(array1);
}
}
@@ -1722,11 +1760,8 @@ public static long copyLarge(final InputStream inputStream, final OutputStream o
* @since 2.2
*/
public static long copyLarge(final InputStream input, final OutputStream output, final long inputOffset, final long length) throws IOException {
- final byte[] buffer = ScratchBufferHolder.getScratchByteArray();
- try {
- return copyLarge(input, output, inputOffset, length, buffer);
- } finally {
- ScratchBufferHolder.releaseScratchByteArray(buffer);
+ try (ScratchBytes scratch = ScratchBytes.get()) {
+ return copyLarge(input, output, inputOffset, length, scratch.array());
}
}
@@ -1797,11 +1832,8 @@ public static long copyLarge(final InputStream input, final OutputStream output,
* @since 1.3
*/
public static long copyLarge(final Reader reader, final Writer writer) throws IOException {
- final char[] buffer = ScratchBufferHolder.getScratchCharArray();
- try {
- return copyLarge(reader, writer, buffer);
- } finally {
- ScratchBufferHolder.releaseScratchCharArray(buffer);
+ try (ScratchChars scratch = IOUtils.ScratchChars.get()) {
+ return copyLarge(reader, writer, scratch.array());
}
}
@@ -1849,11 +1881,8 @@ public static long copyLarge(final Reader reader, final Writer writer, final cha
* @since 2.2
*/
public static long copyLarge(final Reader reader, final Writer writer, final long inputOffset, final long length) throws IOException {
- final char[] buffer = ScratchBufferHolder.getScratchCharArray();
- try {
- return copyLarge(reader, writer, inputOffset, length, buffer);
- } finally {
- ScratchBufferHolder.releaseScratchCharArray(buffer);
+ try (ScratchChars scratch = IOUtils.ScratchChars.get()) {
+ return copyLarge(reader, writer, inputOffset, length, scratch.array());
}
}
@@ -2553,11 +2582,8 @@ public static URL resourceToURL(final String name, final ClassLoader classLoader
* @since 2.0
*/
public static long skip(final InputStream input, final long skip) throws IOException {
- final byte[] buffer = ScratchBufferHolder.getScratchByteArray();
- try {
- return skip(input, skip, () -> buffer);
- } finally {
- ScratchBufferHolder.releaseScratchByteArray(buffer);
+ try (ScratchBytes scratch = ScratchBytes.get()) {
+ return skip(input, skip, scratch::array);
}
}
@@ -2665,18 +2691,16 @@ public static long skip(final Reader reader, final long toSkip) throws IOExcepti
throw new IllegalArgumentException("Skip count must be non-negative, actual: " + toSkip);
}
long remain = toSkip;
- final char[] charArray = ScratchBufferHolder.getScratchCharArray();
- try {
+ try (ScratchChars scratch = IOUtils.ScratchChars.get()) {
+ final char[] chars = scratch.array();
while (remain > 0) {
// See https://issues.apache.org/jira/browse/IO-203 for why we use read() rather than delegating to skip()
- final long n = reader.read(charArray, 0, (int) Math.min(remain, charArray.length));
+ final long n = reader.read(chars, 0, (int) Math.min(remain, chars.length));
if (n < 0) { // EOF
break;
}
remain -= n;
}
- } finally {
- ScratchBufferHolder.releaseScratchCharArray(charArray);
}
return toSkip - remain;
}
diff --git a/src/test/java/org/apache/commons/io/IOCaseTest.java b/src/test/java/org/apache/commons/io/IOCaseTest.java
index 1121b38..54073da 100644
--- a/src/test/java/org/apache/commons/io/IOCaseTest.java
+++ b/src/test/java/org/apache/commons/io/IOCaseTest.java
@@ -30,6 +30,8 @@
import java.io.ObjectOutputStream;
import java.util.Arrays;
+import org.apache.commons.io.IOUtils.ScratchBytes;
+import org.apache.commons.io.IOUtils.ScratchChars;
import org.junit.jupiter.api.Test;
/**
@@ -297,49 +299,48 @@ void test_getName() {
@Test
void test_getScratchByteArray() {
- final byte[] array = IOUtils.ScratchBufferHolder.getScratchByteArray();
- try {
+ final byte[] array;
+ try (ScratchBytes scratch = IOUtils.ScratchBytes.get()) {
+ array = scratch.array();
assert0(array);
Arrays.fill(array, (byte) 1);
// Get another array, while the first is still in use
- final byte[] array2 = IOUtils.ScratchBufferHolder.getScratchByteArray();
- assert0(array2);
- assertNotSame(array, array2);
- } finally {
- // Release first array
- IOUtils.ScratchBufferHolder.releaseScratchByteArray(array);
+ // The test doesn't need the try here but that's the pattern.
+ try (ScratchBytes scratch2 = IOUtils.ScratchBytes.get()) {
+ assertNotSame(scratch, scratch2);
+ final byte[] array2 = scratch2.array();
+ assert0(array2);
+ assertNotSame(array, array2);
+ }
}
// The first array should be reset and reusable
- final byte[] array3 = IOUtils.ScratchBufferHolder.getScratchByteArray();
- try {
+ try (ScratchBytes scratch = IOUtils.ScratchBytes.get()) {
+ final byte[] array3 = scratch.array();
assert0(array3);
assertSame(array, array3);
- } finally {
- IOUtils.ScratchBufferHolder.releaseScratchByteArray(array3);
}
}
@Test
void test_getScratchCharArray() {
- final char[] array = IOUtils.ScratchBufferHolder.getScratchCharArray();
- try {
+ final char[] array;
+ try (ScratchChars scratch = IOUtils.ScratchChars.get()) {
+ array = scratch.array();
assert0(array);
Arrays.fill(array, (char) 1);
// Get another array, while the first is still in use
- final char[] array2 = IOUtils.ScratchBufferHolder.getScratchCharArray();
- assert0(array2);
- assertNotSame(array, array2);
- } finally {
- // Release first array
- IOUtils.ScratchBufferHolder.releaseScratchCharArray(array);
+ // The test doesn't need the try here but that's the pattern.
+ try (ScratchChars scratch2 = IOUtils.ScratchChars.get()) {
+ final char[] array2 = scratch2.array();
+ assert0(array2);
+ assertNotSame(array, array2);
+ }
}
// The first array should be reset and reusable
- final char[] array3 = IOUtils.ScratchBufferHolder.getScratchCharArray();
- try {
+ try (ScratchChars scratch = IOUtils.ScratchChars.get()) {
+ final char[] array3 = scratch.array();
assert0(array3);
assertSame(array, array3);
- } finally {
- IOUtils.ScratchBufferHolder.releaseScratchCharArray(array3);
}
}
