From: Brent Roman <brent@...>
Date: 2004-12-27T09:30:16+09:00
Subject: [PATCH] Long-Standing Bug in the Readline extension

The Readline uses the the GNU readline library's event handling hook
to try to ensure that other ruby threads do not block while it is 
awaiting user keystrokes from STDIN.  I assert that the use of the event 
hook for this purpose is unnecessary and, furthermore, the current 
implementation can set the Thread.critical flag in other threads.

The current threadling_event() hook function is:

static int
readline_event()
{
   CHECK_INTS;
   rb_thread_schedule();  //bug here if Thread.critical !!!
   return 0;
}

If Thread.critical is set when readline is invoked, readline_event() 
will still schedule any other thread that becomes ready.  This can
cause another thread, that was not in a critical section, to interrupt 
one that was.  So, now we have a thread becoming "critical" 
asynchronously.  This leads to some very weird lockups -- some of which 
have been reported on this mailing list.

I think a quick fix might be to remove the unconditional 
rb_thread_schedule() call from readline_event.  But, a better
fix is to remove the readline_event() function entirely.

So, how will we avoid blocking other threads during keyboard reads?
Replace readline_event() with this:

static int
readline_getc (ignored)
   FILE *ignored;
{
   VALUE string = rb_funcall (rb_stdin, rb_intern("sysread"),
						1, INT2FIX(1));
   return RSTRING(string)->ptr[0];  //single byte read
}


We redefine the function that GNU readline uses to read the input stream
to be one based on Ruby's IO#sysread and let Ruby's own IO class
deal with all its threading nastyness.

This has been tested (fairly extensively) in our own multi-threading
motion control application under Ruby 1.6.8.  The same bug appears
to exist in the 1.8.x series, although I have not tried
this patch against 1.8.x.  I'm hoping someone on the list whose
running 1.8.x (everyone?) will test this.


The changes to ext/readline/readline.c are confined to the first few
lines.  Everything from the readline_readline() fn on remains unchanged:



/* readline.c -- GNU Readline module
    Copyright (C) 1997-1998  Shugo Maeda */

#include <errno.h>
#include <stdio.h>
#include <readline/readline.h>
#include <readline/history.h>

#include "ruby.h"
#include "rubysig.h"
#include "rubyio.h"

static VALUE mReadline;

#define TOLOWER(c) (isupper(c) ? tolower(c) : c)

#define COMPLETION_PROC "completion_proc"
#define COMPLETION_CASE_FOLD "completion_case_fold"

#ifndef READLINE_42_OR_LATER
# define rl_filename_completion_function filename_completion_function
# define rl_username_completion_function username_completion_function
# define rl_completion_matches completion_matches
#endif

static int
readline_getc (ignored)
   FILE *ignored;
{
   VALUE string = rb_funcall (rb_stdin, rb_intern("sysread"), 1, 
INT2FIX(1));
   return RSTRING(string)->ptr[0];  //single byte read
}


-- 
  Brent Roman
  mailto:brent@mbari.org  http://www.mbari.org/~brent