Bug 274509 - [WebDriver] The "DELETE /session" command might not complete if the session has already been disconnected.
Summary: [WebDriver] The "DELETE /session" command might not complete if the session h...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebDriver (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: DoNotImportToRadar
Depends on:
Blocks:
 
Reported: 2024-05-22 01:38 PDT by haruhisa.shin
Modified: 2024-05-28 17:03 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description haruhisa.shin 2024-05-22 01:38:24 PDT
The WebDriver may stop responding if the "DELETE /session" command is executed after a browser crash.

If the WebDriver and the browser are working properly, the completionHandler is added to the queue before the "DELETE /session" command is sent to the backend and is called when the response from the backend is received.
However, WebDriver will work the same way even if the socket has already been disconnected due to a crash.
This prevents the completionHandler from being called and the process from completing.

I encountered this problem with the following selenium script.

```
try:
    driver.get(url_of_crash_browser)

except Exception as e:
    print(e)

finally:
    driver.quit()
```

Suppose the browser crashed in `driver.get(url_of_crash_browser)`.
I tried to disconnect the session with `driver.quit()` in the finally block, but this script did not terminate.

To solve this problem, I think SessionHost::sendCommandToBackend() should check if the connection to the backend is valid.
Comment 1 Sam Sneddon [:gsnedders] 2024-05-22 10:49:31 PDT
What WebKit port are you using? Is this with safaridriver? WebKitGTK's WebDriver?
Comment 2 haruhisa.shin 2024-05-22 21:03:12 PDT
Sorry, I forgot to write the port information.
I did check with the wincairo port.
Comment 3 haruhisa.shin 2024-05-23 00:38:34 PDT
Pull request: https://github.com/WebKit/WebKit/pull/28976
Comment 4 haruhisa.shin 2024-05-24 03:00:38 PDT
I reproduced it also with GTK port, so I will describe the procedure just in case.

Steps:
1. Build with GTK port without using flatpak to launch MiniBrowser and WebDriver directly.
2. Open three consoles. I will call them as A, B, and C.
3. Set the environment variable WEBKIT_INSPECTOR_SERVER on console A.
   > export WEBKIT_INSPECTOR_SERVER=127.0.0.1:34567
4. Launch MiniBrowser on console A. The "--automation" option is required.
   > ./MiniBrowser --automation
5. Start WebDriver on consle B.
   > ./WebKitWebDriver -t 127.0.0.1:34567 -p 41900
6. Run the selenium script on console C. The contents are described below.

Expected result:
The 'DONE!' is shown on console C, and the script exits normally.
Actual result:
The 'DONE!' is not shown, and the script does not terminate.


The selenium script used for reproduction is as follows.
The version of selenium is 4.9.0.
Note that the method of setting desired_capabilities is different for 4.10.0 and later.

The "url_of_crash_browser" is the content to crash WebProcess.
If it is difficult to prepare it, you should be able to get the same result by killing WebProcess from another terminal while the script execution is interrupted by time.sleep().
I have confirmed it that way as well.

This script uses webdriver.Remote, but the same result was obtained with webdriver.WebKitGTK.

```
from selenium import webdriver
from selenium.webdriver import WebKitGTKOptions
import time

target_address = '127.0.0.1:34567'
webdriver_host = '127.0.0.1'
webdriver_port = 41900

options = webdriver.WebKitGTKOptions()
driver = webdriver.Remote(
	command_executor="http://{0}:{1}".format(webdriver_host, webdriver_port),
	desired_capabilities={
		'webkitgtk:browserOptions':{
			'targetAddress': target_address
		}
	}
)

try:
	driver.get(url_of_crash_browser)

except Exception as e:
	print(e)

finally:
	driver.quit()
	print('DONE!')
```
Comment 5 EWS 2024-05-28 17:03:52 PDT
Committed 279417@main (827066c642dd): <https://commits.webkit.org/279417@main>

Reviewed commits have been landed. Closing PR #28976 and removing active labels.