[rb] Add check of process existence before polling for exit of process

[yoshoku]

Description

In the stop method of Selenium::WebDriver::ServiceManager, I would like to add a check if the process exists before calling the poll_for_exit method of the instance variable @process.

Motivation and Context

The simplest error reproduction is as follows:

$ bundle exec irb
irb(main):001:0> require 'selenium-webdriver'
irb(main):002:0> service = Selenium::WebDriver::Service.chrome
irb(main):003:0> service_manager = Selenium::WebDriver::ServiceManager.new(service)
irb(main):004:0> service_manager.stop
/Users/yoshoku/selenium/rb/lib/selenium/webdriver/common/service_manager.rb:65:in `stop': 
undefined method `poll_for_exit' for nil:NilClass (NoMethodError)
...

I am using selenium-webdriver with capybara and parallel_tests, and sometimes I get this error.

In the stop_server private method of Selenium::WebDriver::ServiceManager, the existence of the process is checked at the beginning. I think that the same check is also necessary before calling the poll_for_exit method.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[CLAassistant]

All committers have signed the CLA.

[p0deje]

@yoshoku Thank you for the contribution!

I am using selenium-webdriver with capybara and parallel_tests, and sometimes I get this error.

I would rather try to understand why you get this error first. By the time this code is called, the @process should already be defined as #start should be called before #stop. Can you provide more details on the original error you experience?

[luke-hill]

@p0deje how about instead using the safe operator here? That would maybe cover any extenuating use case and not need the extra addition?

[yoshoku]

@p0deje Thank you for your interest.
In my environment, this error occurs when the socket lock fails. If the socket_lock.locked method raises Error::WebDriverError, exit_hook calls the stop method without creating a process.
https://github.com/SeleniumHQ/selenium/blob/selenium-4.0.0/rb/lib/selenium/webdriver/common/service_manager.rb#L52

For example, this error can be reproduced by rewriting the lock private method as follows:
https://github.com/SeleniumHQ/selenium/blob/selenium-4.0.0/rb/lib/selenium/webdriver/common/socket_lock.rb#L49

def lock
  raise Error::WebDriverError
end

$ git clone git@github.com:teamcapybara/capybara.git
$ cd capybara
$ bundle install
$ bundle exec rspec spec/selenium_spec_chrome.rb
...
/foobar/gems/selenium-webdriver-4.0.3/lib/selenium/webdriver/common/service_manager.rb:65:in `stop': undefined method `poll_for_exit' for nil:NilClass (NoMethodError)
  from /foobar/gems/selenium-webdriver-4.0.3/lib/selenium/webdriver/common/platform.rb:155:in `block in exit_hook'
...

[p0deje]

@yoshoku Thank you for the issue and fix, I've implemented a bit different solution in e3e492a so hopefully you won't see this problem in the next version.