Skip to content

Commit 2707d1f

Browse files
authored
Prevent classloader memory leak in ScratchBytes/ScratchChars (#804)
Commit 0698bd9 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.
1 parent 0698bd9 commit 2707d1f

File tree

1 file changed

+22
-12
lines changed

1 file changed

+22
-12
lines changed

src/main/java/org/apache/commons/io/IOUtils.java

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,9 @@ static final class ScratchBytes implements AutoCloseable {
156156
* [0] boolean in use.
157157
* [1] byte[] buffer.
158158
*/
159-
private static final ThreadLocal<Object[]> LOCAL = ThreadLocal.withInitial(() -> new Object[] { false, new ScratchBytes(byteArray()) });
159+
private static final ThreadLocal<Object[]> LOCAL = ThreadLocal.withInitial(() -> new Object[] { false, byteArray() });
160+
161+
private static final ScratchBytes INSTANCE = new ScratchBytes(null);
160162

161163
/**
162164
* Gets the internal byte array buffer.
@@ -170,27 +172,30 @@ static ScratchBytes get() {
170172
return new ScratchBytes(byteArray());
171173
}
172174
holder[0] = true;
173-
return (ScratchBytes) holder[1];
175+
return INSTANCE;
174176
}
175177

178+
/**
179+
* The buffer, or null if using the thread-local buffer.
180+
*/
176181
private final byte[] buffer;
177182

178183
private ScratchBytes(final byte[] buffer) {
179184
this.buffer = buffer;
180185
}
181186

182187
byte[] array() {
183-
return buffer;
188+
return buffer != null ? buffer : (byte[]) LOCAL.get()[1];
184189
}
185190

186191
/**
187192
* If the buffer is the internal array, clear and release it for reuse.
188193
*/
189194
@Override
190195
public void close() {
191-
final Object[] holder = LOCAL.get();
192-
if (buffer == ((ScratchBytes) holder[1]).buffer) {
193-
Arrays.fill(buffer, (byte) 0);
196+
if (buffer == null) {
197+
final Object[] holder = LOCAL.get();
198+
Arrays.fill((byte[]) holder[1], (byte) 0);
194199
holder[0] = false;
195200
}
196201
}
@@ -220,7 +225,9 @@ static final class ScratchChars implements AutoCloseable {
220225
* [0] boolean in use.
221226
* [1] char[] buffer.
222227
*/
223-
private static final ThreadLocal<Object[]> LOCAL = ThreadLocal.withInitial(() -> new Object[] { false, new ScratchChars(charArray()) });
228+
private static final ThreadLocal<Object[]> LOCAL = ThreadLocal.withInitial(() -> new Object[] { false, charArray() });
229+
230+
private static final ScratchChars INSTANCE = new ScratchChars(null);
224231

225232
/**
226233
* Gets the internal char array buffer.
@@ -234,27 +241,30 @@ static ScratchChars get() {
234241
return new ScratchChars(charArray());
235242
}
236243
holder[0] = true;
237-
return (ScratchChars) holder[1];
244+
return INSTANCE;
238245
}
239246

247+
/**
248+
* The buffer, or null if using the thread-local buffer.
249+
*/
240250
private final char[] buffer;
241251

242252
private ScratchChars(final char[] buffer) {
243253
this.buffer = buffer;
244254
}
245255

246256
char[] array() {
247-
return buffer;
257+
return buffer != null ? buffer : (char[]) LOCAL.get()[1];
248258
}
249259

250260
/**
251261
* If the buffer is the internal array, clear and release it for reuse.
252262
*/
253263
@Override
254264
public void close() {
255-
final Object[] holder = LOCAL.get();
256-
if (buffer == ((ScratchChars) holder[1]).buffer) {
257-
Arrays.fill(buffer, (char) 0);
265+
if (buffer == null) {
266+
final Object[] holder = LOCAL.get();
267+
Arrays.fill((char[]) holder[1], (char) 0);
258268
holder[0] = false;
259269
}
260270
}

0 commit comments

Comments
 (0)