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

[rb] Waiting till Socket connection completed #6909

Merged
merged 2 commits into from
Mar 7, 2019
Merged

[rb] Waiting till Socket connection completed #6909

merged 2 commits into from
Mar 7, 2019

Conversation

N0xFF
Copy link
Contributor

@N0xFF N0xFF commented Feb 1, 2019

After select we should use getsockopt to read the SO_ERROR option at
level SOL_SOCKET to determine that connection completed successfully. Otherwise in connect_nonblock we can get EISCONN error.

http://man7.org/linux/man-pages/man2/connect.2.html


This change is Reviewable

After `select` we should use `getsockopt` to read the SO_ERROR option at
level SOL_SOCKET to determine that connection completed successfully

http://man7.org/linux/man-pages/man2/connect.2.html
@N0xFF
Copy link
Contributor Author

N0xFF commented Feb 1, 2019

This is reason why I all time get error

Errno::ECONNREFUSED: Failed to open TCP connection to 127.0.0.1:9515 (Connection refused - connect(2) for "127.0.0.1" port 9515)

and can't run Ruby on Rails system tests on WSL.

@p0deje p0deje added the C-rb label Feb 4, 2019
@twalpole
Copy link
Contributor

While I believe this PR is technically more correct, I fail to see how it fixes any Errno::ECONNREFUSED issue you're having?

@N0xFF
Copy link
Contributor Author

N0xFF commented Feb 28, 2019

begin
sock.connect_nonblock sockaddr
rescue Errno::EINPROGRESS
retry if IO.select(nil, [sock], nil, CONNECT_TIMEOUT)

Errno::EINPROGRESS error means that connection in progress and we need to check status to make a decision. We check that socked is writable via IO.select and retry connection. But connection is not ready and next we got Errno::EALREADY error.

It was caught by

rescue *CONNECTED_ERRORS
# yay!
end
sock.close
true

So listening? returns true. That allow commands by webdriver. But connection still not ready and finally we got Errno::ECONNREFUSED that not handled.

@N0xFF
Copy link
Contributor Author

N0xFF commented Mar 1, 2019

Now we check conn_completed? otherwise raise Errno::ECONNREFUSED. It will be catched in

rescue *NOT_CONNECTED_ERRORS
sock.close if sock
WebDriver.logger.debug("polling for socket on #{[@host, @port].inspect}")
false
end

So listening? returns false and listening? is retry with new connection in with_timeout block.
def connected?
with_timeout { listening? }
end

@twalpole
Copy link
Contributor

twalpole commented Mar 7, 2019

Ok - makes sense -- do you have time to fix the conflicts here so it's mergeable?

# Conflicts:
#	rb/lib/selenium/webdriver/common/socket_poller.rb
@N0xFF
Copy link
Contributor Author

N0xFF commented Mar 7, 2019

Done. But after merge I found other problem #7010

@twalpole twalpole merged commit e8bd56e into SeleniumHQ:master Mar 7, 2019
@twalpole
Copy link
Contributor

twalpole commented Mar 7, 2019

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants