Bug #4555

[PATCH] ext/socket/init.c: rsock_connect retries on interrupt

Added by Eric Wong about 3 years ago. Updated almost 3 years ago.

[ruby-core:35621]
Status:Closed
Priority:Normal
Assignee:Motohiro KOSAKI
Category:ext
Target version:2.0.0
ruby -v:- Backport:

Description

=begin
Otherwise I get the following error in test/openssl/test_ssl.rb:

testverifyresult(OpenSSL::TestSSL):
Errno::EINTR: Interrupted system call - connect(2)
/tmp/ruby/test/openssl/testssl.rb:338:in initialize'
/tmp/ruby/test/openssl/test_ssl.rb:338:in
new'
/tmp/ruby/test/openssl/test
ssl.rb:338:in block in test_verify_result'
/tmp/ruby/test/openssl/test_ssl.rb:117:in
call'
/tmp/ruby/test/openssl/testssl.rb:117:in start_server'
/tmp/ruby/test/openssl/test_ssl.rb:330:in
test
verify_result'

This bug is made more noticeable by r31230, though it always
existed before.

Fwiw, I think all the waitconnectable() logic incorrectly uses
the except
fds parameter of select() and rbiowait_writable()
should be used here...

=end

0001-ext-socket-init.c-rsock_connect-retries-on-interrupt.patch Magnifier - patch to fix the issue (1.48 KB) Eric Wong, 04/06/2011 08:13 AM

Associated revisions

Revision 31405
Added by Motohiro KOSAKI almost 3 years ago

  • ext/socket/init.c (rsock_connect): add to care EINTR. based on a patch from Eric Wong at [Bug #4555]

History

#1 Updated by Yui NARUSE about 3 years ago

  • Status changed from Open to Assigned
  • Assignee set to Motohiro KOSAKI
  • Priority changed from High to Normal

=begin

=end

#2 Updated by Motohiro KOSAKI about 3 years ago

=begin
Hi

I'm not convinced why we can safely call select() when we get EINTR. IOW,
Why don't you choose following patch?

Index: ext/socket/init.c

--- ext/socket/init.c (revision 31258)
+++ ext/socket/init.c (working copy)
@@ -383,6 +383,12 @@
status = (int)BLOCKINGREGIONFD(func, &arg);
if (status < 0) {
switch (errno) {
+ case EINTR:
+#if defined(ERESTART)
+ case ERESTART:
+#endif
+ continue;
+
case EAGAIN:
#ifdef EINPROGRESS
case EINPROGRESS:

=end

#3 Updated by Eric Wong about 3 years ago

=begin
Your patch looks reasonable to me, but maybe some platforms break under it...

I was trying to emulate rbiowaitwritable() logic which calls
rb
threadfdwritable() (which wraps select() if there are multiple threads).
Maybe avoiding select() in all EINTR cases will work, I am under the impression
a lot of the select()-wrapping calls in Ruby are relics of the old green threading
and can be removed.

=end

#4 Updated by Motohiro KOSAKI almost 3 years ago

  • ruby -v changed from ruby 1.9.3dev (2011-04-05 trunk 31241) [x86_64-linux] to -

Your patch looks reasonable to me, but maybe some platforms break under it...

Can you please clarify this? Which platform break and why?

I was trying to emulate rbiowaitwritable() logic which calls
rb
threadfdwritable() (which wraps select() if there are multiple threads).
Maybe avoiding select() in all EINTR cases will work, I am under the impression
a lot of the select()-wrapping calls in Ruby are relics of the old green threading
and can be removed.

I missed your point. If anyone set non-blocking flag and call socket#connect,
the connect call in rsock_connect() can return non-blocking related error. We
have to care it even though we only have single thread. Am I missing something?

#5 Updated by Eric Wong almost 3 years ago

KOSAKI Motohiro kosaki.motohiro@gmail.com wrote:

Your patch looks reasonable to me, but maybe some platforms break under it...

Can you please clarify this? Which platform break and why?

Nevermind, see below:

I was trying to emulate rbiowaitwritable() logic which calls
rb
threadfdwritable() (which wraps select() if there are multiple threads).
Maybe avoiding select() in all EINTR cases will work, I am under the impression
a lot of the select()-wrapping calls in Ruby are relics of the old green threading
and can be removed.

I missed your point. If anyone set non-blocking flag and call socket#connect,
the connect call in rsock_connect() can return non-blocking related error. We
have to care it even though we only have single thread. Am I missing something?

You're correct. I was mistaken and thought rsockconnect() was intended
to be retrying and emulate a blocking operation (like IO#write even if
O
NONBLOCK is set)

--
Eric Wong

#6 Updated by Motohiro KOSAKI almost 3 years ago

  • Status changed from Assigned to Closed

Also available in: Atom PDF