1
Fork 0
mirror of https://git.savannah.gnu.org/git/guile.git synced 2025-04-29 19:30:36 +02:00

Remove AC_SYS_RESTARTABLE_SYSCALLS and related code

As the Autoconf documentation says, "These days portable programs
[...] should not rely on `HAVE_RESTARTABLE_SYSCALLS', since nowadays
whether a system call is restartable is a dynamic issue, not a
configuration-time issue."

In other words, if we ever rely on HAVE_RESTARTABLE_SYSCALLS, we are
at the mercy of any code that Guile happens to be linked with, because
that code could install a signal handler without the SA_RESTART flag,
and then a Guile system call could unexpectedly return EINTR.

The readline part of this goes back to this problem report:
http://sources.redhat.com/ml/guile/2000-05/msg00177.html; and is an
excellent example of the above paragraph.  It was noted during the
discussion that undefining HAVE_RESTARTABLE_SYSCALLS would fix the
problem, but that solution wasn't adopted - I guess because Guile was
still using cooperative threads then (not pthreads) and so there was a
significant concern (whether founded or not) that not using
restartable syscalls (where available) could lead to a loss of
performance.

Now Guile's default mode of operation is with pthreads, where we
already don't assume that HAVE_RESTARTABLE_SYSCALLS is reliable, so
there is no possible further performance loss.  And in any case we
really have no choice, if we want correct operation.

Thanks to Sylvain Beucler for reporting this and suggesting the fix.

* configure.in (AC_SYS_RESTARTABLE_SYSCALLS): Removed.

* doc/ref/posix.texi (Signals): Remove statement that Guile always
  sets SA_RESTART flag.

* guile-readline/configure.in (GUILE_SIGWINCH_SA_RESTART_CLEARED):
  Remove this setting, together with its test code.
  (HAVE_RL_PRE_INPUT_HOOK): Remove this setting and its code, as no
  longer needed.

* guile-readline/readline.c (sigwinch_enable_restart): Removed.
  (scm_init_readline): Remove setting of rl_pre_input_hook.

* libguile/_scm.h (SCM_SYSCALL): Remove the definition that relies on
  HAVE_RESTARTABLE_SYSCALLS.

* libguile/scmsigs.c (scm_sigaction_for_thread): Don't always set the
  SA_RESTART flag if available.  Update docstring accordingly.
  (scm_init_scmsigs): Remove code that sets SA_RESTART flag for all
  signals.

* THANKS: Add Sylvain.
This commit is contained in:
Neil Jerram 2009-06-18 20:35:45 +01:00
parent 0d646345f4
commit 0ebbcf43c4
7 changed files with 3 additions and 158 deletions

1
THANKS
View file

@ -24,6 +24,7 @@ For fixes or providing information which led to a fix:
David Allouche
Martin Baulig
Fabrice Bauzac
Sylvain Beucler
Carlo Bramini
Rob Browning
Adrian Bunk

View file

@ -1054,18 +1054,6 @@ if test $guile_cv_localtime_cache = yes; then
AC_DEFINE(LOCALTIME_CACHE, 1, [Define if localtime caches the TZ setting.])
fi
dnl Test whether system calls are restartable by default on the
dnl current system. If they are not, we put a loop around every system
dnl call to check for EINTR (see SCM_SYSCALL) and do not attempt to
dnl change from the default behaviour. On the other hand, if signals
dnl are restartable then the loop is not installed and when libguile
dnl initialises it also resets the behaviour of each signal to cause a
dnl restart (in case a different runtime had a different default
dnl behaviour for some reason: e.g., different versions of linux seem
dnl to behave differently.)
AC_SYS_RESTARTABLE_SYSCALLS
if test "$enable_regex" = yes; then
if test "$ac_cv_header_regex_h" = yes ||
test "$ac_cv_header_rxposix_h" = yes ||

View file

@ -1909,10 +1909,6 @@ for termination, not stopping.
If a signal occurs while in a system call, deliver the signal then
restart the system call (as opposed to returning an @code{EINTR} error
from that call).
Guile always enables this flag where available, no matter what
@var{flags} are specified. This avoids spurious error returns in low
level operations.
@end defvar
The return value is a pair with information about the old handler as

View file

@ -54,77 +54,6 @@ dnl install paren matching on the Guile command line (when using
dnl readline for input), so it's completely optional.
AC_CHECK_FUNCS(rl_get_keymap)
dnl Check for rl_pre_input_hook. This is more complicated because on
dnl some systems (HP/UX), the linker wont let us treat
dnl rl_pre_input_hook as a function when it really is a function
dnl pointer.
AC_MSG_CHECKING([for rl_pre_input_hook])
AC_CACHE_VAL(ac_cv_var_rl_pre_input_hook,
[AC_TRY_LINK([
#include <stdio.h>
#include <readline/readline.h>
], [
rl_pre_input_hook = 0;
],
ac_cv_var_rl_pre_input_hook=yes,
ac_cv_var_rl_pre_input_hook=no)])
AC_MSG_RESULT($ac_cv_var_rl_pre_input_hook)
if test $ac_cv_var_rl_pre_input_hook = yes; then
AC_DEFINE(HAVE_RL_PRE_INPUT_HOOK,1,
[Define if rl_pre_input_hook is available.])
fi
AC_MSG_CHECKING(if readline clears SA_RESTART flag for SIGWINCH)
AC_CACHE_VAL(guile_cv_sigwinch_sa_restart_cleared,
AC_TRY_RUN([#include <signal.h>
#include <stdio.h>
#include <readline/readline.h>
int
hook ()
{
struct sigaction action;
sigaction (SIGWINCH, NULL, &action);
rl_cleanup_after_signal();
/* exit with 0 if readline disabled SA_RESTART */
exit (action.sa_flags & SA_RESTART);
}
int
main ()
{
struct sigaction action;
sigaction (SIGWINCH, NULL, &action);
action.sa_flags |= SA_RESTART;
sigaction (SIGWINCH, &action, NULL);
/* Give readline something to read. Otherwise, it might hang, for
example when run as a background process with job control.
*/
rl_instream = fopen ("/dev/null", "r");
if (rl_instream == NULL)
{
perror ("/dev/null");
exit (1);
}
rl_pre_input_hook = hook;
readline ("");
}],
guile_cv_sigwinch_sa_restart_cleared=yes,
guile_cv_sigwinch_sa_restart_cleared=no,
guile_cv_sigwinch_sa_restart_cleared=yes))
AC_MSG_RESULT($guile_cv_sigwinch_sa_restart_cleared)
if test $guile_cv_sigwinch_sa_restart_cleared = yes; then
AC_DEFINE(GUILE_SIGWINCH_SA_RESTART_CLEARED, 1,
[Define if readline disables SA_RESTART.])
fi
AC_CACHE_CHECK([for rl_getc_function pointer in readline],
ac_cv_var_rl_getc_function,
[AC_TRY_LINK([

View file

@ -530,26 +530,6 @@ match_paren (int x, int k)
}
#endif /* HAVE_RL_GET_KEYMAP */
#if defined (HAVE_RL_PRE_INPUT_HOOK) && defined (GUILE_SIGWINCH_SA_RESTART_CLEARED)
/* Readline disables SA_RESTART on SIGWINCH.
* This code turns it back on.
*/
static int
sigwinch_enable_restart (void)
{
#ifdef HAVE_SIGINTERRUPT
siginterrupt (SIGWINCH, 0);
#else
struct sigaction action;
sigaction (SIGWINCH, NULL, &action);
action.sa_flags |= SA_RESTART;
sigaction (SIGWINCH, &action, NULL);
#endif
return 0;
}
#endif
#endif /* HAVE_RL_GETC_FUNCTION */
void
@ -569,9 +549,6 @@ scm_init_readline ()
#endif
rl_basic_word_break_characters = "\t\n\"'`;()";
rl_readline_name = "Guile";
#if defined (HAVE_RL_PRE_INPUT_HOOK) && defined (GUILE_SIGWINCH_SA_RESTART_CLEARED)
rl_pre_input_hook = sigwinch_enable_restart;
#endif
reentry_barrier_mutex = scm_permanent_object (scm_make_mutex ());
scm_init_opts (scm_readline_options,

View file

@ -79,20 +79,6 @@
#include "libguile/modules.h"
#include "libguile/inline.h"
/* SCM_SYSCALL retries system calls that have been interrupted (EINTR).
However this can be avoided if the operating system can restart
system calls automatically. We assume this is the case if
sigaction is available and SA_RESTART is defined; they will be used
when installing signal handlers.
*/
#ifdef HAVE_RESTARTABLE_SYSCALLS
#if ! SCM_USE_PTHREAD_THREADS /* However, don't assume SA_RESTART
works with pthreads... */
#define SCM_SYSCALL(line) line
#endif
#endif
#ifndef SCM_SYSCALL
#ifdef vms
# ifndef __GNUC__

View file

@ -306,10 +306,8 @@ SCM_DEFINE (scm_sigaction_for_thread, "sigaction", 1, 3, 0,
"a scheme procedure has been specified, that procedure will run\n"
"in the given @var{thread}. When no thread has been given, the\n"
"thread that made this call to @code{sigaction} is used.\n"
"Flags can "
"optionally be specified for the new handler (@code{SA_RESTART} will\n"
"always be added if it's available and the system is using restartable\n"
"system calls.) The return value is a pair with information about the\n"
"Flags can optionally be specified for the new handler.\n"
"The return value is a pair with information about the\n"
"old handler as described above.\n\n"
"This interface does not provide access to the \"signal blocking\"\n"
"facility. Maybe this is not needed, since the thread support may\n"
@ -333,14 +331,7 @@ SCM_DEFINE (scm_sigaction_for_thread, "sigaction", 1, 3, 0,
csig = scm_to_signed_integer (signum, 0, NSIG-1);
#if defined(HAVE_SIGACTION)
#if defined(SA_RESTART) && defined(HAVE_RESTARTABLE_SYSCALLS)
/* don't allow SA_RESTART to be omitted if HAVE_RESTARTABLE_SYSCALLS
is defined, since libguile would be likely to produce spurious
EINTR errors. */
action.sa_flags = SA_RESTART;
#else
action.sa_flags = 0;
#endif
if (!SCM_UNBNDP (flags))
action.sa_flags |= scm_to_int (flags);
sigemptyset (&action.sa_mask);
@ -713,29 +704,6 @@ scm_init_scmsigs ()
#else
orig_handlers[i] = SIG_ERR;
#endif
#ifdef HAVE_RESTARTABLE_SYSCALLS
/* If HAVE_RESTARTABLE_SYSCALLS is defined, it's important that
signals really are restartable. don't rely on the same
run-time that configure got: reset the default for every signal.
*/
#ifdef HAVE_SIGINTERRUPT
siginterrupt (i, 0);
#elif defined(SA_RESTART)
{
struct sigaction action;
sigaction (i, NULL, &action);
if (!(action.sa_flags & SA_RESTART))
{
action.sa_flags |= SA_RESTART;
sigaction (i, &action, NULL);
}
}
#endif
/* if neither siginterrupt nor SA_RESTART are available we may
as well assume that signals are always restartable. */
#endif
}
scm_c_define ("NSIG", scm_from_long (NSIG));