* [PATCH 1/2] ext/socket/init.c: use SOCK_NONBLOCK if available
@ 2015-05-12 0:06 Eric Wong
2015-05-12 0:06 ` [PATCH 2/2] socket: accept_nonblock supports "nonblock: false" kwarg Eric Wong
0 siblings, 1 reply; 2+ messages in thread
From: Eric Wong @ 2015-05-12 0:06 UTC (permalink / raw)
To: spew
This saves a system call by allowing us to use SOCK_NONBLOCK in
Linux when accept4 is available.
Note: I do not agree accept_nonblock should always make accepted
sockets non-blocking, and will propose a future API to allow
controlling whether accepted sockets are non-blocking or not
regardless of how they were created.
* ext/socket/init.c (cloexec_accept): support nonblock flag and
use SOCK_NONBLOCK if possible
* ext/socket/init.c (rsock_s_accept_nonblock): update cloexec_accept call
* ext/socket/init.c (accept_blocking): ditto for blocking
* test/socket/test_nonblock.rb: check nonblock? on accepted socket
---
ext/socket/init.c | 21 +++++++++++++++++----
test/socket/test_nonblock.rb | 4 ++++
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/ext/socket/init.c b/ext/socket/init.c
index 50d3f86..317dd67 100644
--- a/ext/socket/init.c
+++ b/ext/socket/init.c
@@ -471,7 +471,8 @@ make_fd_nonblock(int fd)
}
static int
-cloexec_accept(int socket, struct sockaddr *address, socklen_t *address_len)
+cloexec_accept(int socket, struct sockaddr *address, socklen_t *address_len,
+ int nonblock)
{
int ret;
socklen_t len0 = 0;
@@ -485,11 +486,21 @@ cloexec_accept(int socket, struct sockaddr *address, socklen_t *address_len)
#ifdef SOCK_CLOEXEC
flags |= SOCK_CLOEXEC;
#endif
+#ifdef SOCK_NONBLOCK
+ if (nonblock) {
+ flags |= SOCK_NONBLOCK;
+ }
+#endif
ret = accept4(socket, address, address_len, flags);
/* accept4 is available since Linux 2.6.28, glibc 2.10. */
if (ret != -1) {
if (ret <= 2)
rb_maygvl_fd_fix_cloexec(ret);
+#ifndef SOCK_NONBLOCK
+ if (nonblock) {
+ make_fd_nonblock(ret);
+ }
+#endif
if (address_len && len0 < *address_len) *address_len = len0;
return ret;
}
@@ -503,6 +514,9 @@ cloexec_accept(int socket, struct sockaddr *address, socklen_t *address_len)
if (ret == -1) return -1;
if (address_len && len0 < *address_len) *address_len = len0;
rb_maygvl_fd_fix_cloexec(ret);
+ if (nonblock) {
+ make_fd_nonblock(ret);
+ }
return ret;
}
@@ -521,7 +535,7 @@ rsock_s_accept_nonblock(int argc, VALUE *argv, VALUE klass, rb_io_t *fptr,
rb_secure(3);
rb_io_set_nonblock(fptr);
- fd2 = cloexec_accept(fptr->fd, (struct sockaddr*)sockaddr, len);
+ fd2 = cloexec_accept(fptr->fd, (struct sockaddr*)sockaddr, len, 1);
if (fd2 < 0) {
switch (errno) {
case EAGAIN:
@@ -539,7 +553,6 @@ rsock_s_accept_nonblock(int argc, VALUE *argv, VALUE klass, rb_io_t *fptr,
rb_sys_fail("accept(2)");
}
rb_update_max_fd(fd2);
- make_fd_nonblock(fd2);
return rsock_init_sock(rb_obj_alloc(klass), fd2);
}
@@ -553,7 +566,7 @@ static VALUE
accept_blocking(void *data)
{
struct accept_arg *arg = data;
- return (VALUE)cloexec_accept(arg->fd, arg->sockaddr, arg->len);
+ return (VALUE)cloexec_accept(arg->fd, arg->sockaddr, arg->len, 0);
}
VALUE
diff --git a/test/socket/test_nonblock.rb b/test/socket/test_nonblock.rb
index 98de00e..5268252 100644
--- a/test/socket/test_nonblock.rb
+++ b/test/socket/test_nonblock.rb
@@ -1,5 +1,6 @@
begin
require "socket"
+ require "io/nonblock"
rescue LoadError
end
@@ -24,6 +25,9 @@ class TestSocketNonblock < Test::Unit::TestCase
s, sockaddr = serv.accept_nonblock
end
assert_equal(Socket.unpack_sockaddr_in(c.getsockname), Socket.unpack_sockaddr_in(sockaddr))
+ if s.respond_to?(:nonblock?)
+ assert s.nonblock?, 'accepted socket is non-blocking'
+ end
ensure
serv.close if serv
c.close if c
--
EW
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH 2/2] socket: accept_nonblock supports "nonblock: false" kwarg
2015-05-12 0:06 [PATCH 1/2] ext/socket/init.c: use SOCK_NONBLOCK if available Eric Wong
@ 2015-05-12 0:06 ` Eric Wong
0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2015-05-12 0:06 UTC (permalink / raw)
To: spew
An application wanting to do non-blocking accept may want to
create a blocking socket, allow
If this patch is accepted, I'd also like to support the opposite
for blocking accept calls:
a = s.accept(nonblock: true) # blocking accept syscall
a.nonblock? # => true
s.nonblock? # => false
---
ext/socket/init.c | 14 ++++++++++----
test/socket/test_nonblock.rb | 14 ++++++++++++++
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/ext/socket/init.c b/ext/socket/init.c
index 317dd67..0bafec4 100644
--- a/ext/socket/init.c
+++ b/ext/socket/init.c
@@ -29,7 +29,7 @@ VALUE rb_cSOCKSSocket;
#endif
int rsock_do_not_reverse_lookup = 1;
-static VALUE sym_exception, sym_wait_readable;
+static VALUE sym_exception, sym_wait_readable, sym_nonblock;
void
rsock_raise_socket_error(const char *reason, int error)
@@ -526,16 +526,21 @@ rsock_s_accept_nonblock(int argc, VALUE *argv, VALUE klass, rb_io_t *fptr,
{
int fd2;
int ex = 1;
+ int nonblock = 1;
VALUE opts = Qnil;
rb_scan_args(argc, argv, "0:", &opts);
- if (!NIL_P(opts) && Qfalse == rb_hash_lookup2(opts, sym_exception, Qundef))
- ex = 0;
+ if (!NIL_P(opts)) {
+ if (Qfalse == rb_hash_lookup2(opts, sym_exception, Qundef))
+ ex = 0;
+ if (Qfalse == rb_hash_lookup2(opts, sym_nonblock, Qundef))
+ nonblock = 0;
+ }
rb_secure(3);
rb_io_set_nonblock(fptr);
- fd2 = cloexec_accept(fptr->fd, (struct sockaddr*)sockaddr, len, 1);
+ fd2 = cloexec_accept(fptr->fd, (struct sockaddr*)sockaddr, len, nonblock);
if (fd2 < 0) {
switch (errno) {
case EAGAIN:
@@ -639,4 +644,5 @@ rsock_init_socket_init(void)
#undef rb_intern
sym_exception = ID2SYM(rb_intern("exception"));
sym_wait_readable = ID2SYM(rb_intern("wait_readable"));
+ sym_nonblock = ID2SYM(rb_intern("nonblock"));
}
diff --git a/test/socket/test_nonblock.rb b/test/socket/test_nonblock.rb
index 5268252..a862784 100644
--- a/test/socket/test_nonblock.rb
+++ b/test/socket/test_nonblock.rb
@@ -27,6 +27,20 @@ class TestSocketNonblock < Test::Unit::TestCase
assert_equal(Socket.unpack_sockaddr_in(c.getsockname), Socket.unpack_sockaddr_in(sockaddr))
if s.respond_to?(:nonblock?)
assert s.nonblock?, 'accepted socket is non-blocking'
+
+ tf = [ true, false ]
+ assert_equal(tf, tf.map do |nb|
+ begin
+ b = Socket.new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
+ b.connect(serv.getsockname)
+ IO.select([serv])
+ a, _= serv.accept_nonblock(nonblock: nb)
+ a.nonblock?
+ ensure
+ a.close if a
+ b.close if b
+ end
+ end)
end
ensure
serv.close if serv
--
EW
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-05-12 0:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 0:06 [PATCH 1/2] ext/socket/init.c: use SOCK_NONBLOCK if available Eric Wong
2015-05-12 0:06 ` [PATCH 2/2] socket: accept_nonblock supports "nonblock: false" kwarg Eric Wong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).