0.10.0-SNAPSHOT is leaking heap memory via ThreadLocal

[psiroky]

I was testing the latest bits from the mvnd-0.10.x branch (built locally) and noticed an increasing heap usage with every consequent build (of the same project). I am using the Quarkus project (tag 2.16.1.Final) and the simplest call I could think of -- just mvnd clean.

Here is the heap usage summary. It went from ~100MiB to ~1.8GiB in a matter of 20 seconds or so. The "steps" correspond to different mvnd clean executions. My expectation would be that the heap usage goes back to the original number once the last execution finishes (actually it should probably go back to the original value after each execution - I noticed the System.gc() call in the code -- so technically that can be ignored by JVM, but experimentally it should clean-up the heap after every execution).

The heap dump summary looks like this:

Drilling down to the biggest portion of the occupied heap, we get this:

It seems like this is "caused" by the upgrade to Maven 3.9.0. I was not able to reproduce the same with mvnd 0.9.0, which bundles Maven 3.8.7.

My investigation so far:

• I was looking through the call tree of the biggest memory consumer (see above) and it looks like there is maybe some cache or so, being held via ThreadLocal and it is never cleared, accumulating data over subsequent builds
• I stumbled upon apache/maven@b762fa9 which changes private MavenProject currentProject; into private ThreadLocal<MavenProject> currentProject = new ThreadLocal<>(); -- this looks somewhat suspicious, but I haven't yet confirmed if this is related or not

My environment:

• mvnd built from branch mvnd-0.10.x, specifically the commit c65f9fe
• JDK 19.0.2
• Fedora Linux 37 (x64, kernel 6.1.14)

[psiroky]

Here are some more details from my investigation:

• the ThreadLocal instances are held by the Thread-0 thread, which is the main "accept" thread

  maven-mvnd/daemon/src/main/java/org/mvndaemon/mvnd/daemon/Server.java

  Line 216 in c65f9fe

  new DaemonThread(this::accept).start();

  This thread waits for messages (build requests) from the client and then runs the Maven build. As far as I can tell this thread is only stopped when the daemon is stopping, so the ThreadLocals are not freed-up, unless explicit ThreadLocal.remove() is called.
• I am almost sure the change in Maven 3.9.0 (apache/maven@b762fa9) is related here, because the private ThreadLocal<MavenProject> currentProject is never freed-up. There is somewhat circular dependency - MavenProject references MavenSession (indirectly, via private ProjectBuildingRequest projectBuilderConfiguration) and MavenSession references the current MavenProject via a thread local.
• there is probably a fix in Maven itself which could be applied, but generally this could happen at any point in the future again (since Maven is usually just "one-shot" executed, so these kind of things are super hard to spot). I am wondering if we could do something on the Maven daemon side to avoid this?

I did the following dummy experiment (I know this would need further refinement, but I just wanted to see if this approach helps or not).

Instead of running the build (method call client(socket)) on the main "accept" thread

private void accept() {
        try {
            while (true) {
                try (SocketChannel socket = this.socket.accept()) {
                    client(socket);
                }
            }
        } catch (Throwable t) {
            LOGGER.error("Error running daemon loop", t);
        }
    }

I created a new thread for each request and then just waited for that thread to finish:

private void accept() {
        try {
            while (true) {
                try (SocketChannel socket = this.socket.accept()) {
                    Thread thread = new Thread(() -> client(socket));
                    thread.start();
                    thread.join();
                }
            }
        } catch (Throwable t) {
            LOGGER.error("Error running daemon loop", t);
        }
    }

I am well aware of the issues here (the catch is basically useless as any exception would be thrown inside that other thread). Also additional classloader configuration may be needed.

In any case, this seems to "solve" the memory leak I am seeing. Once the thread finishes (stops), the ThreadLocals are no longer accessible and thus are eligible for collection. Something like this would also "guard" against similar issues in the future I believe (since the thread itself is thrown away, anything it references would be thrown away as well). There may some other issues with this approach I am not seeing though.

[gnodet]

@psiroky your analysis looks good to me, and the solution too. Could you raise a PR and handle the exception so that it's at least logged ?