Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Terminate WebDrivers, always #593

Merged
merged 1 commit into from
Mar 16, 2023
Merged

Conversation

thc202
Copy link
Contributor

@thc202 thc202 commented Mar 16, 2023

Change CrawlTaskConsumer to always close the crawler, in a finally block.
Change WebDriverBackedEmbeddedBrowser to close/quit the browser in a different thread than the one being called to avoid interruptions, which could prevent the WebDriver (e.g. chromedriver, geckodriver) from being terminated, leading to process leaks.
Quit the browser directly instead of using WebDriverManager when it does not contain any as the WebDriver might not have been created through it (e.g. when using custom Provider<EmbeddedBrowser>).


Downstream change: zaproxy#20.


Sample exception when failed to quit the WebDriver because of the interrupt:

NettyWebSocket initial request interrupted
java.lang.InterruptedException
	at java.base/java.util.concurrent.CompletableFuture.reportGet(CompletableFuture.java:385)
	at java.base/java.util.concurrent.CompletableFuture.get(CompletableFuture.java:1999)
	at org.openqa.selenium.remote.http.netty.NettyWebSocket.<init>(NettyWebSocket.java:99)
	at org.openqa.selenium.remote.http.netty.NettyWebSocket.lambda$create$3(NettyWebSocket.java:128)
	at org.openqa.selenium.remote.http.netty.NettyClient.openSocket(NettyClient.java:107)
	at org.openqa.selenium.devtools.Connection.<init>(Connection.java:78)
	at org.openqa.selenium.firefox.FirefoxDriver.maybeGetDevTools(FirefoxDriver.java:269)
	at org.openqa.selenium.remote.RemoteWebDriver.quit(RemoteWebDriver.java:438)
	at org.openqa.selenium.firefox.FirefoxDriver.quit(FirefoxDriver.java:323)
…

which prevents both WebDriver and browser to quit.

Change `CrawlTaskConsumer` to always close the crawler, in a finally
block.
Change `WebDriverBackedEmbeddedBrowser` to close/quit the browser in a
different thread than the one being called to avoid interruptions,
which could prevent the WebDriver (e.g. chromedriver, geckodriver) from
being terminated, leading to process leaks.
Quit the browser directly instead of using WebDriverManager when it
does not contain any as the WebDriver might not have been created
through it (e.g. when using custom `Provider<EmbeddedBrowser>`).

Signed-off-by: thc202 <thc202@gmail.com>
Copy link
Member

@amesbah amesbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@amesbah amesbah merged commit b2af34f into crawljax:master Mar 16, 2023
@thc202 thc202 deleted the webdriver-leaks branch March 17, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants