This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git
The following commit(s) were added to refs/heads/master by this push:
new 2707d1fd7 Prevent classloader memory leak in
`ScratchBytes`/`ScratchChars` (#804)
2707d1fd7 is described below
commit 2707d1fd70e22ae3675508d294bb93046e02b209
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Sun Oct 19 03:07:19 2025 +0200
Prevent classloader memory leak in `ScratchBytes`/`ScratchChars` (#804)
Commit 0698bd9eafb2d20fb85f4ea4f695db1b702dcef2 introduced convenient
`AutoCloseable` usage for `ScratchBytes` and `ScratchChars`. However, it also
introduced a **classloader memory leak risk** in application server
environments by storing custom wrapper instances directly in a `ThreadLocal`.
This PR keeps the ergonomic `AutoCloseable` pattern while eliminating the
classloader leak risk:
* Store **only primitive buffers** (`byte[]` / `char[]`) in the
`ThreadLocal`, not custom classes.
* Introduce two types of `ScratchBytes` / `ScratchChars` instances:
* **Global instance** (`buffer == null`) that fetches its buffer from the
`ThreadLocal`.
* **Reentrant instances** (`buffer != null`) for nested usage without
interfering with shared buffers.
**Note:** While this revision keeps the readability of using the
`AutoCloseable` API, it also introduces a performance regression compared to
the original #801 design: retrieving a buffer now requires two `ThreadLocal`
lookups: once in `get()` and once in `array()`. The original design avoided
this overhead intentionally. Since these classes are package-private and used
in performance-sensitive paths, we should carefully weigh the trade-off between
API convenience and runtime cost.
---
src/main/java/org/apache/commons/io/IOUtils.java | 34 +++++++++++++++---------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/src/main/java/org/apache/commons/io/IOUtils.java
b/src/main/java/org/apache/commons/io/IOUtils.java
index 5d3d7cc58..5a005fa67 100644
--- a/src/main/java/org/apache/commons/io/IOUtils.java
+++ b/src/main/java/org/apache/commons/io/IOUtils.java
@@ -156,7 +156,9 @@ static final class ScratchBytes implements AutoCloseable {
* [0] boolean in use.
* [1] byte[] buffer.
*/
- private static final ThreadLocal<Object[]> LOCAL =
ThreadLocal.withInitial(() -> new Object[] { false, new
ScratchBytes(byteArray()) });
+ private static final ThreadLocal<Object[]> LOCAL =
ThreadLocal.withInitial(() -> new Object[] { false, byteArray() });
+
+ private static final ScratchBytes INSTANCE = new ScratchBytes(null);
/**
* Gets the internal byte array buffer.
@@ -170,9 +172,12 @@ static ScratchBytes get() {
return new ScratchBytes(byteArray());
}
holder[0] = true;
- return (ScratchBytes) holder[1];
+ return INSTANCE;
}
+ /**
+ * The buffer, or null if using the thread-local buffer.
+ */
private final byte[] buffer;
private ScratchBytes(final byte[] buffer) {
@@ -180,7 +185,7 @@ private ScratchBytes(final byte[] buffer) {
}
byte[] array() {
- return buffer;
+ return buffer != null ? buffer : (byte[]) LOCAL.get()[1];
}
/**
@@ -188,9 +193,9 @@ byte[] array() {
*/
@Override
public void close() {
- final Object[] holder = LOCAL.get();
- if (buffer == ((ScratchBytes) holder[1]).buffer) {
- Arrays.fill(buffer, (byte) 0);
+ if (buffer == null) {
+ final Object[] holder = LOCAL.get();
+ Arrays.fill((byte[]) holder[1], (byte) 0);
holder[0] = false;
}
}
@@ -220,7 +225,9 @@ static final class ScratchChars implements AutoCloseable {
* [0] boolean in use.
* [1] char[] buffer.
*/
- private static final ThreadLocal<Object[]> LOCAL =
ThreadLocal.withInitial(() -> new Object[] { false, new
ScratchChars(charArray()) });
+ private static final ThreadLocal<Object[]> LOCAL =
ThreadLocal.withInitial(() -> new Object[] { false, charArray() });
+
+ private static final ScratchChars INSTANCE = new ScratchChars(null);
/**
* Gets the internal char array buffer.
@@ -234,9 +241,12 @@ static ScratchChars get() {
return new ScratchChars(charArray());
}
holder[0] = true;
- return (ScratchChars) holder[1];
+ return INSTANCE;
}
+ /**
+ * The buffer, or null if using the thread-local buffer.
+ */
private final char[] buffer;
private ScratchChars(final char[] buffer) {
@@ -244,7 +254,7 @@ private ScratchChars(final char[] buffer) {
}
char[] array() {
- return buffer;
+ return buffer != null ? buffer : (char[]) LOCAL.get()[1];
}
/**
@@ -252,9 +262,9 @@ char[] array() {
*/
@Override
public void close() {
- final Object[] holder = LOCAL.get();
- if (buffer == ((ScratchChars) holder[1]).buffer) {
- Arrays.fill(buffer, (char) 0);
+ if (buffer == null) {
+ final Object[] holder = LOCAL.get();
+ Arrays.fill((char[]) holder[1], (char) 0);
holder[0] = false;
}
}
