[
https://issues.apache.org/jira/browse/LANG-1839?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136111#comment-18136111
]
Gary D. Gregory commented on LANG-1839:
---------------------------------------
[~rajugupta]
Thanks for the report but note that the Java class above doesn't compile.
> ToStringStyle.VISITED ThreadLocal grows without bound when a
> ReflectionToStringBuilder is created but toString() is never called (memory
> leak since 3.21.0)
> -----------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: LANG-1839
> URL: https://issues.apache.org/jira/browse/LANG-1839
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.builder.*
> Affects Versions: 3.21.0, 3.22.0
> Environment: Apache Commons Lang 3.21.0 (also reproduced on master
> 3.22.0-SNAPSHOT, commit 61837eb), OpenJDK 17, macOS
> Reporter: Raju Gupta
> Priority: Major
>
> Since 3.21.0, \{{ToStringStyle.VISITED}} (a ThreadLocal \{{IdentityHashMap}})
> can grow without bound and cause an \{{OutOfMemoryError}}. It happens when a
> \{{ReflectionToStringBuilder}} is constructed but \{{toString()}} is never
> called on it. Reported by Oli Gillespie on the user list ("Memory leak in
> ReflectionToStringBuilder/ToStringStyle"). I reproduced it on current master
> (3.22.0-SNAPSHOT).
> h3. Cause
> * Constructing a \{{ToStringBuilder}} calls \{{style.appendStart(...)}},
> which calls \{{register(object)}}. The root object is added to the
> thread-local \{{REGISTRY}} in the constructor.
> * The root is only removed by \{{unregister(object)}}, called from
> \{{appendEnd}} (i.e. \{{toString()}}). A builder that is never stringified
> leaves its root in \{{REGISTRY}} for the life of the thread.
> * \{{VISITED}} is only cleared in \{{unregister()}}, and only when
> \{{REGISTRY.isEmpty()}}.
> * \{{RecursiveToStringStyle.appendInternal}} calls \{{markVisited(value)}}
> for every object it renders.
> Once one orphaned builder exists on a thread, \{{REGISTRY}} is never empty
> again, so \{{VISITED}} is never cleared. Every later \{{toString()}} on that
> thread adds its whole object graph to \{{VISITED}}, and none of it is
> released.
> The stale \{{REGISTRY}} entry was already possible before 3.21.0, but it cost
> one small entry per thread. \{{VISITED}} ties the cleanup of a large per-call
> set to that set being empty, which turns the orphan into unbounded growth.
> h3. Real-world trigger
> {code:java}
> log.debug("...", new ReflectionToStringBuilder(...));
> {code}
> The builder is created eagerly. If debug logging is disabled, \{{toString()}}
> is never called.
> h3. Reproducer
> {code:java}
> import org.apache.commons.lang3.builder.*;
> import java.lang.reflect.Field;
> import java.util.*;
> public class VisitedLeak {
> static class Graph \{ List<Object> items = new ArrayList<>() {{ for (int
> i = 0; i < 999; i++) add(new Object()); }}; }
> static int size(String field) throws Exception {
> Field f = ToStringStyle.class.getDeclaredField(field);
> f.setAccessible(true);
> return ((Map<?, ?>) ((ThreadLocal<?>) f.get(null)).get()).size();
> }
> public static void main(String[] args) throws Exception {
> if (args.length == 0) new ReflectionToStringBuilder(new Graph(), new
> RecursiveToStringStyle()); // start leak
> for (int i = 0; i <= 1000; i++) {
> if (i % 100 == 0) System.out.printf("Invocation %d:
> REGISTRY.size=%d VISITED.size=%d%n", i, size("REGISTRY"), size("VISITED"));
> new ReflectionToStringBuilder(new Graph(), new
> RecursiveToStringStyle()).toString();
> }
> }
> }
> {code}
> Run without arguments (leaks) and with \{{clean}} (control, no orphan
> builder).
> h3. Actual result
> * Leak run: \{{Invocation 1000: REGISTRY.size=1 VISITED.size=1000000}}
> * Control run: \{{REGISTRY.size=0 VISITED.size=0}}
> * With \{{-Xmx64m}} the leak run fails with \{{java.lang.OutOfMemoryError:
> Java heap space}} in under 2,000 iterations.
> h3. Expected result
> {\{VISITED}} is empty after every top-level \{{toString()}} completes,
> regardless of builders that were created but never stringified.
> h3. Suggested fix
> Clear \{{VISITED}} when the outermost
> \{{ReflectionToStringBuilder.toString()}} completes (in a \{{finally}}),
> instead of relying on \{{REGISTRY}} becoming empty. A nested call always
> starts after the nested value was marked visited, so an empty \{{VISITED}} on
> entry identifies the top-level call and no new state is needed. I have a
> patch and a regression test and can open a GitHub PR.
> h3. Workaround
> Don't construct the builder unless \{{toString()}} will be called, e.g. guard
> with \{{if (log.isDebugEnabled())}} or pass a lazy supplier.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
