[jira] [Commented] (CXF-9146) MemoryLeak caused by ThreadLocalClientState when used with the HttpClientConduit

["Andriy Redko (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CXF-9146?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17963254#comment-17963254 ] 

Andriy Redko commented on CXF-9146:
-----------------------------------

Thanks a lot for the comprehensive analysis [~cheesemaster] , I will take a closer look shortly (indeed, ThreadLocalClientState was created 15 years ago). I have lowered this ticket to major - this is absolutely nothing critical about it (no crashes, OOMs, data corruption, etc)

> MemoryLeak caused by ThreadLocalClientState when used with the HttpClientConduit
> --------------------------------------------------------------------------------
>
>                 Key: CXF-9146
>                 URL: https://issues.apache.org/jira/browse/CXF-9146
>             Project: CXF
>          Issue Type: Bug
>            Reporter: Eric
>            Priority: Major
>         Attachments: CxfClientMemoryLeakTest.java, image-2025-06-10-19-04-43-021.png, image-2025-06-10-19-05-03-593.png
>
>
> {color:#000000}Is defined as following:{color}
> {code:java}
> public class ThreadLocalClientState implements ClientState {
>        private Map<Thread, LocalClientState> state = Collections.synchronizedMap(new WeakHashMap<Thread, LocalClientState>()); {code}
> ... which does not make any sense to me, as is this is just ThreadLocal built at home, a class which as been in the JDK for a very long time.. A WeakHashMap of Threads might emulate ThreadLocal,  but it can cause unexpected memory leaks, because the Weak-Keys are strongly held until the WeakHashMap is accessed for the next time.
>  
> Let's take this simple example:
> {code:java}
> var mb = 1024 * 1024;
> var rt = Runtime.getRuntime();
> var entries = new WeakHashMap<Object, byte[][]>();
> var keys = new ArrayList<>();
> System.gc();
> System.out.printf("Memory before Memory Load: %s mb%n", (rt.totalMemory() - rt.freeMemory()) / mb);
> IntStream.range(0, 100).mapToObj(i -> new Date(i + 1000)).forEach(key -> {
>     keys.add(key);
>     entries.put(key, new byte[1][10 * mb]);
> });
> System.gc();
> System.out.printf("Memory after Memory Load: %s mb%n", (rt.totalMemory() - rt.freeMemory()) / mb);
> keys.clear();
> System.gc();
> System.out.printf("Memory after Memory Keys clear: %s mb%n", (rt.totalMemory() - rt.freeMemory()) / mb);
> System.out.printf("Map Size: %s%n", entries.size());
> System.gc();
> System.out.printf("Memory after first Memory Map access: %s mb%n", (rt.totalMemory() - rt.freeMemory()) / mb);{code}
>  
> This leads to the following output:
> {noformat}
> Memory before Memory Load: 13 mb
> Memory after Memory Load: 1213 mb
> Memory after Memory Keys clear: 1213 mb
> Map Size: 0
> Memory after first Memory Map access: 13 mb
> {noformat}
>  
> Please ignore my inaccurate measurements with System.gc() without using JMH, but when looking up the source for WeakHashMap then it becomes obvious that no cleanup is technically possible until the WeakHashMap is deferenced for the next time for a method of the Map interface. This is a scenario which should rarely happen in a production enviroment, but we have seen it at least once being the cause of an OutOfMemoryError wenn many threads in in parallel executed requests which returned very large Response objects in memory.
> Therefore, and especially when thinking of the future of virtual threads and scoped values, I would advise to the refactor the code so that it internally uses a simple Treadlocal from now on instead of the WeakHashMap.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)