search

MachinePublishers / jBrowserDriver

A programmable, embeddable web browser driver compatible with the Selenium WebDriver spec -- headless, WebKit-based, pure Java
Other
809 stars 143 forks source link

Thread leaks #270

Closed tomcashman closed 7 years ago

tomcashman commented 7 years ago

There appear to be 3 threads leaking inside the driver. I've submitted pull request #269 that fixes one of them. The other 2 threads seem to be unnamed threads. We receive the following warnings when redeploying our tomcat webapp.

WARNING: The web application [app] appears to have started a thread named [Thread-10] but has failed to stop it. This is very likely to create a memory leak. Stack trace of thread:
 java.lang.Thread.sleep(Native Method)
 com.machinepublishers.jbrowserdriver.JBrowserDriver$2.run(JBrowserDriver.java:365)
 java.lang.Thread.run(Thread.java:745)
Apr 07, 2017 5:09:53 PM org.apache.catalina.loader.WebappClassLoaderBase clearReferencesThreads
WARNING: The web application [app] appears to have started a thread named [Thread-20] but has failed to stop it. This is very likely to create a memory leak. Stack trace of thread:
 java.lang.Thread.sleep(Native Method)
 com.machinepublishers.jbrowserdriver.JBrowserDriver$2.run(JBrowserDriver.java:365)
 java.lang.Thread.run(Thread.java:745)

I've had difficulty tracing through the code to figure out where they could be. I originally thought it could be related to AjaxListener since that thread didn't have a name so I set a name on its thread but the same errors occurred so this can be ruled out.

hollingsworthd commented 7 years ago

There are 2 other threads in JBrowserDriver.java that might be causing this. Based on the stack trace it might be the "heartbeat" thread. That was added in https://github.com/MachinePublishers/jBrowserDriver/pull/251 ... That thread should terminate when the browser is quit. Although there's a 5 second sleep between checking so maybe it's exceeding a timeout for Tomcat to wait for threads to end. Or maybe the browser isn't quitting properly?

It probably can't be the threads in any of the other classes because those are all in the child process so would be outside of the app server's concern. (And those processes will get terminated when the parent process ends)

hollingsworthd commented 7 years ago

@tomcashman this is released now in v0.17.7