Permalink
Browse files

Fix EAGAIN being raised erroneously

  • Loading branch information...
1 parent 5ac1fe0 commit eb5062c2070ccdacc150d20f634b2ff9ce77708e @pietern pietern committed Dec 1, 2011
Showing with 57 additions and 5 deletions.
  1. +7 −1 CHANGELOG.md
  2. +20 −4 ext/hiredis_ext/connection.c
  3. +30 −0 test/connection_test.rb
View
@@ -1,4 +1,10 @@
-### 0.4.2
+### 0.4.3
+
+* Fix bug that caused EAGAIN to be raised after the cumulative time spent
+ waiting for the socket to become readable/writable exceeded the
+ connection-wide timeout.
+
+### 0.4.2 (unreleased)
* Use patched version of hiredis to support multi bulk depth of 2.
@@ -72,12 +72,20 @@ static VALUE connection_parent_context_alloc(VALUE klass) {
return Data_Wrap_Struct(klass, parent_context_mark, parent_context_free, pc);
}
-static int __wait_readable(int fd, struct timeval *timeout, int *isset) {
+static int __wait_readable(int fd, const struct timeval *timeout, int *isset) {
+ struct timeval to;
+ struct timeval *toptr = NULL;
fd_set fds;
FD_ZERO(&fds);
FD_SET(fd, &fds);
- if (rb_thread_select(fd + 1, &fds, NULL, NULL, timeout) < 0) {
+ /* rb_thread_select modifies the passed timeval, so we pass a copy */
+ if (timeout != NULL) {
+ memcpy(&to, timeout, sizeof(to));
+ toptr = &to;
+ }
+
+ if (rb_thread_select(fd + 1, &fds, NULL, NULL, toptr) < 0) {
return -1;
}
@@ -88,12 +96,20 @@ static int __wait_readable(int fd, struct timeval *timeout, int *isset) {
return 0;
}
-static int __wait_writable(int fd, struct timeval *timeout, int *isset) {
+static int __wait_writable(int fd, const struct timeval *timeout, int *isset) {
+ struct timeval to;
+ struct timeval *toptr = NULL;
fd_set fds;
FD_ZERO(&fds);
FD_SET(fd, &fds);
- if (rb_thread_select(fd + 1, NULL, &fds, NULL, timeout) < 0) {
+ /* rb_thread_select modifies the passed timeval, so we pass a copy */
+ if (timeout != NULL) {
+ memcpy(&to, timeout, sizeof(to));
+ toptr = &to;
+ }
+
+ if (rb_thread_select(fd + 1, NULL, &fds, NULL, toptr) < 0) {
return -1;
}
@@ -310,6 +310,36 @@ def test_eagain_on_write_followed_by_remote_drain
end
end
end
+
+ def test_no_eagain_after_cumulative_wait_exceeds_timeout
+ listen do |server|
+ hiredis.connect("localhost", 6380)
+ hiredis.timeout = 10_000
+
+ begin
+ thread = Thread.new do
+ loop do
+ sleep(0.001)
+ server.write("+ok\r\n")
+ end
+ end
+
+ # The read timeout for this connection is 10 milliseconds.
+ # To compensate for the overhead of parsing the reply and the chance
+ # not having to wait because the reply is already present in the OS
+ # buffers, continue until we have waited at least 5x the timeout.
+ waited = 0
+ while waited < 50_000
+ t1 = Time.now
+ hiredis.read
+ t2 = Time.now
+ waited += (t2 - t1) * 1_000_000
+ end
+ ensure
+ thread.kill
+ end
+ end
+ end
end
if defined?(Hiredis::Ruby::Connection)

0 comments on commit eb5062c

Please sign in to comment.