ruby-core

Hello,

Recently I encountered a problem that I think others have experienced
before that has to do with using gets on standard input and its side
effect of blocking all other ruby threads on win32.  The problem has to
do with the fact that the existing code will simply indicate that there
is data on any non-socket file handle that is passed to select (which
includes stdin).  This leads to a blocking call being made to read (via
getc) that prevents other ruby threads from running until data has been
written to the console.  To help address this problem, I'm submitting a
patch that seems to address the issue, though perhaps there is a better
way to go about integrating it (I'll leave that up to the developers).

Here's a sample script that illustrates the problem:

Thread.new { 
	100.times { |x|
		puts "#{Time.now.to_i}: #{x}"
		$stdout.flush
		sleep 1
	}
}

begin
	puts "#{Time.now.to_i}: reading..."
	$stdout.flush
	puts "#{Time.now.to_i}: read in: #{$stdin.gets}"
	$stdout.flush
end while true

If you run this script on windows using the existing native build of 
the ruby interpreter, the output is:

1131086953: 0
1131086953: reading...  <- blocking call issued
h                       <- h<enter> typed
1131086953: read in: h
1131086960: 1           <- thread gets scheduled once
1131086960: reading...  <- blocking call issued
< blocks until more data is sent >

If you run this script using a native interpreter that has been 
compiled with my patch, the output is more along the lines of what
you would expect:

1131087075: 0
1131087075: reading...
1131087076: 1              <- thread continues to run
1131087077: 2
1131087078: 3
input                      <- input<enter> typed
1131087075: read in: input
1131087079: 4
1131087079: reading...
1131087080: 5              <- thread continues to run
1131087081: 6
1131087082: 7
1131087083: 8
1131087084: 9
1131087085: 10

Now, this patch doesn't address all of the issues.  One problem that
remains (which I consider separate from this patch) is that once data
is typed, all ruby threads will be blocked until enter is pressed
(when gets is being used).  This could be addressed by a subsequent
patch which would affect a separate area of code.

Please let me know if there are any questions or feedback.  There are
most likely some optimizations that could be made to this, but this
should illustrate a fundamental approach to fixing this problem.

Matt

Attachments (1)

ruby_win32_stdin_select.patch (2.53 KB, text/x-diff)

--- ruby-1.8.3-orig/win32/win32.c	2005-09-14 08:41:02.000000000 -0500
+++ ruby-1.8.3/win32/win32.c	2005-11-04 00:39:21.448000000 -0600
@@ -1800,8 +1800,6 @@
     return -1;
 }
 
-#undef FD_SET
-
 void
 rb_w32_fdset(int fd, fd_set *set)
 {
@@ -1821,8 +1819,6 @@
     }
 }
 
-#undef FD_CLR
-
 void
 rb_w32_fdclr(int fd, fd_set *set)
 {
@@ -1841,8 +1837,6 @@
     }
 }
 
-#undef FD_ISSET
-
 int
 rb_w32_fdisset(int fd, fd_set *set)
 {
@@ -1905,22 +1899,46 @@
     fd_set trap;
 #endif /* USE_INTERRUPT_WINSOCK */
     int file_nfds;
+    int stdin_data = 0, stdin_fd;
 
     if (!NtSocketsInitialized) {
 	StartSockets();
     }
+
+    // If stdin is being polled, check to see if it has data using
+    // WaitForSingleObjectEx since passing it directly to select (if we were to
+    // do such a thing) would be futile and would simply return data present all
+    // the time (thus leading to potential ruby thread blocking conditions).
+    if ((rd) && 
+        ((stdin_fd = fileno(stdin)) >= 0) &&
+        (FD_ISSET(stdin_fd, rd)))
+    {
+        // Remove stdin from the read set. 
+        FD_CLR(stdin_fd, rd);
+
+        RUBY_CRITICAL({
+            // If the wait routine indicates that there is data waiting on
+            // stdin, then set the flag that tells the latter call to select
+            // that we need to manually set stdin in the read set after select
+            // is called.
+            if (WaitForSingleObjectEx(GetStdHandle(STD_INPUT_HANDLE),
+                    0, TRUE) == WAIT_OBJECT_0)
+                stdin_data = 1;
+        });
+    }
+
     r = 0;
     if (rd && rd->fd_count > r) r = rd->fd_count;
     if (wr && wr->fd_count > r) r = wr->fd_count;
     if (ex && ex->fd_count > r) r = ex->fd_count;
     if (nfds > r) nfds = r;
-    if (nfds == 0 && timeout) {
+    if (nfds == 0 && timeout && !stdin_data) {
 	Sleep(timeout->tv_sec * 1000 + timeout->tv_usec / 1000);
 	return 0;
     }
     file_nfds = extract_file_fd(rd, &file_rd);
     file_nfds += extract_file_fd(wr, &file_wr);
-    if (file_nfds)
+    if (file_nfds && !stdin_data)
     {
 	// assume normal files are always readable/writable
 	// fake read/write fd_set and return value
@@ -1945,10 +1963,22 @@
 	if (r == SOCKET_ERROR) {
 	    errno = map_errno(WSAGetLastError());
 	}
+       
+	// If stdin had data, then set it in the read set and update the return
+	// value accordingly.
+	if (stdin_data)
+	{
+	    FD_SET(stdin_fd, rd);
+	    r = (r < 0) ? 1 : r + 1;
+	}
     });
     return r;
 }
 
+#undef FD_SET
+#undef FD_CLR
+#undef FD_ISSET
+
 static void
 StartSockets ()
 {

Thread

Prev Next

In This Thread

Prev Next