From 9b5b5b393f5085d9ec24154e01e06e69d0bbd956 Mon Sep 17 00:00:00 2001
From: "Piotr P. Karwasz" <pkarwasz-github@apache.org>
Date: Sat, 18 Oct 2025 23:23:21 +0200
Subject: [PATCH] Prevent classloader memory leak in
 `ScratchBytes`/`ScratchChars`

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.
---
 .../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 5d3d7cc5850..5a005fa6792 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;
             }
         }