[🚀 Feature]: `Service` shutting down via `__del__` is unintuitive (as `Remote` only consumes the service url)

[GideonBear]

Feature and motivation

I recently faced a weird bug where returning from a function would cause the browser to close and Selenium to complain they couldn't reach the driver. Turned out, my Service was garbage collected (and quitting it via __del__) after I had used its service_url.

from selenium import webdriver
from selenium.webdriver.chrome import service


def get_driver():
    webdriver_service = service.Service("operadriver")
    webdriver_service.start()

    options = webdriver.ChromeOptions()
    options.add_experimental_option('w3c', True)

    driver = webdriver.Remote(webdriver_service.service_url, options=options)

    return driver


driver = get_driver()
driver.get("https://example.com")

While in hindsight it is expected behavior based on the code (I did not pass webdriver_service, I passed a string I got from it), it's not very weird (especially if you don't know the internals of how webdrivers work) to overlook the .service_url and assume that Remote would set webdriver_service as an instance attribute, keeping it from being garbage collected. This behavior is in my opinion very unintuitive. A possible solution would be to only allow passing the whole Service into Remote instead of just the service_url (and setting it as an instance attribute), but this would be backwards-incompatible. Maybe add another method for passing the whole Service (or overload the original method), and encourage using that (for example by raising a warning when the service_url parameter is accessed) instead of the service_url? I don't have a good proposal, but I thought I would submit an issue anyway, since it cost me a lot of time to debug.

After writing this, it came to mind that using Service and Remote directly might not have been intended anyway. The readme of webdriver-manager told me to do it this way, probably because Opera was deprecated. I'll submit it anyway, but if this is the case, it's probably a lot less important.

Usage example

Not applicable

[github-actions[bot]]

@GideonBear, thank you for creating this issue. We will troubleshoot it as soon as we can.

---

Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

[github-actions[bot]]

This issue is stale because it has been open 280 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[GideonBear]

Bump

[github-actions[bot]]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[GideonBear]

Bump

[cgoldberg]

I don't particularly like using __del__ at all, but we have used this to shut down the driver for many years without much complaint. I don't see any actual issue this is causing or anything actionable to be done, so I am closing this.

[GideonBear]

I don't particularly like using __del__ at all, but we have used this to shut down the driver for many years without much complaint. I don't see any actual issue this is causing or anything actionable to be done, so I am closing this.

@cgoldberg To clarify: My actual issue this is causing is that dropping the Service before the Remote breaks the Remote. My actionable proposal is passing the Service into the Remote instead of just the service_url, which allows the Remote to "own" the Service, preventing it from being dropped; by e.g. Remote.from_service and Remote.from_service_url.

What exactly do you mean by "I don't see"? Because this was all stated in my original issue.

[cgoldberg]

@GideonBear feel free to submit a PR with your suggestion (that is backwards compatible)