vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

Threads are not stopped between server reloads #17731

Open mshabarov opened 1 year ago

mshabarov commented 1 year ago

Description of the bug

When hot deploying a Java changes in Flow or Hilla applications, the following warn messages appear in the server logs:

o.a.c.loader.WebappClassLoaderBase       : The web application [ROOT] appears to have started a thread named [HttpClient-50-Worker-1] but has failed to stop it. This is very likely to create a memory leak. Stack trace of thread:
 java.base@17.0.6/jdk.internal.misc.Unsafe.park(Native Method)
 java.base@17.0.6/java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:252)
 java.base@17.0.6/java.util.concurrent.SynchronousQueue$TransferStack.transfer(SynchronousQueue.java:401)
 java.base@17.0.6/java.util.concurrent.SynchronousQueue.poll(SynchronousQueue.java:903)
 java.base@17.0.6/java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1061)
 java.base@17.0.6/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1122)
 java.base@17.0.6/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
 java.base@17.0.6/java.lang.Thread.run(Thread.java:833)

A possible workaround to suppress the warning would be to supply an Executor to the HttpClient.Builder, and shutdown this Executor in the main method of the app. The permanent solution might be to place shutdown into VaadinService::destroy.

Also, we might consider to reuse those threads over redeploys to avoid spending time on waiting for the executor to be closed.

Expected behavior

No warnings in the logs.

Minimal reproducible example

Download starter project from https://start.vaadin.com, run it, apply Java changes and recompile to trigger a reload.

Versions

caalador commented 1 year ago

I only get [JNA Cleaner] on the first reload and never again for future reloads for changes.

mshabarov commented 1 year ago

I get the same for Flow applications with Vaadin 24.2.0.rc1.

But with Hilla base starter with Hilla 2.3.0.alpha11 and 2.3-SNAPSHOT I get a lot of warnings as given in the description.

EDIT: can get these warnings with Vite dev server enabled for Flow application. So the culprit is a dev server codes that connects to the dev server.

mshabarov commented 1 year ago

In Flow core there are three places of using HttpClient that I could find:

StatisticsSender can be switched off by config parameter and even then I got warnings. DefaultFileDownloader downloads Node.js and during server reloads it isn't triggered.

The last one, ViteWebsocketConnection is a culprit. WebSocket connection is closed via

        CompletableFuture<WebSocket> closeRequest = clientWebSocket
                .sendClose(CloseCodes.NORMAL_CLOSURE.getCode(), "");
        closeRequest.get();

but probably some threads for sending messages in send() aren't completed due to waiting for data to be sent to Vite.

mshabarov commented 1 year ago

Tried with timeout, but no success:

            send.get(100, TimeUnit.MILLISECONDS);
        } catch (TimeoutException e) {
            getLogger().debug("Message not sent because of timeout", e);
        }
tarekoraby commented 12 months ago

When hot deploying a Java changes in Flow or Hilla applications

For me, the error occurs on the first run of a fresh skeleton project even without any code changes.

tltv commented 11 months ago

I also only get warning about [JNA Cleaner] thread, and also just once after first reload. It originates from thread from com.sun.jna.iternal.Cleaner class which is from the latest JNA 5.13.0.

Looks like a fix for it in 5.14.0 will be released soon (in few weeks) so I suggest to wait for it: https://groups.google.com/g/jna-users/c/OjCQEHaDagI

tarekoraby commented 10 months ago

https://github.com/java-native-access/jna/releases/tag/5.14.0 is now released. Can we upgrade?

mshabarov commented 10 months ago

Upgraded in the license checker https://github.com/vaadin/license-checker-vaadin10/pull/121. Flow doesn't seem to have oshi/jna explicit version.