This closes patch:

[ 960406 ] unblock signals in threads although the changes do not correspond exactly to any patch attached to that report. Non-main threads no longer have all signals masked. A different interface to readline is used. The handling of signals inside calls to PyOS_Readline is now rather different. These changes are all a bit scary! Review and cross-platform testing much appreciated.
2024-07-21 02:05:18 +00:00 · 2004-07-07 17:44:12 +00:00 · 2004-07-07 17:44:12 +00:00 · 30ea2f223f
parent e3c330b42a
commit 30ea2f223f
10 changed files with 1576 additions and 3095 deletions
--- a/Misc/ACKS
+++ b/Misc/ACKS
@ -327,6 +327,7 @@ Hannu Krosing
 Andrew Kuchling
 Vladimir Kushnir
 Cameron Laird
+Andrew Langmead
 Detlef Lannert
 Soren Larsen
 Piers Lauder
--- a/Modules/readline.c
+++ b/Modules/readline.c
@ -656,6 +656,66 @@ setup_readline(void)
 #endif
 }

+/* Wrapper around GNU readline that handles signals differently. */
+
+
+#if defined(HAVE_RL_CALLBACK) && defined(HAVE_SELECT)
+
+static	char *completed_input_string;
+static void
+rlhandler(char *text)
+{
+	completed_input_string = text;
+	rl_callback_handler_remove();
+}
+
+extern PyThreadState* _PyOS_ReadlineTState;
+
+static char *
+readline_until_enter_or_signal(char *prompt, int *signal)
+{
+	char * not_done_reading = "";
+	fd_set selectset;
+
+	*signal = 0;
+#ifdef HAVE_RL_CATCH_SIGNAL
+	rl_catch_signals = 0;
+#endif
+
+	rl_callback_handler_install (prompt, rlhandler);
+	FD_ZERO(&selectset);
+	FD_SET(fileno(rl_instream), &selectset);
+	
+	completed_input_string = not_done_reading;
+
+	while(completed_input_string == not_done_reading) {
+		int has_input;
+
+		has_input = select(fileno(rl_instream) + 1, &selectset,
+				   NULL, NULL, NULL);
+		if(has_input > 0) {
+			rl_callback_read_char();
+		}
+		else if (errno == EINTR) {
+			int s;
+			PyEval_RestoreThread(_PyOS_ReadlineTState);
+			s = PyErr_CheckSignals();
+			PyThreadState_Swap(NULL);	
+			if (s < 0) {
+				rl_free_line_state();
+				rl_cleanup_after_signal();
+				rl_callback_handler_remove();
+				*signal = 1;
+				completed_input_string = NULL;
+			}
+		}
+	}
+
+	return completed_input_string;
+}
+
+
+#else

 /* Interrupt handler */

@ -669,14 +729,13 @@ onintr(int sig)
 }


-/* Wrapper around GNU readline that handles signals differently. */
-
 static char *
-call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
+readline_until_enter_or_signal(char *prompt, int *signal)
 {
-	size_t n;
-	char *p, *q;
 	PyOS_sighandler_t old_inthandler;
+	char *p;
+    
+	*signal = 0;

 	old_inthandler = PyOS_setsig(SIGINT, onintr);
 	if (setjmp(jbuf)) {
@ -685,8 +744,24 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
 		sigrelse(SIGINT);
 #endif
 		PyOS_setsig(SIGINT, old_inthandler);
+		*signal = 1;
 		return NULL;
 	}
+	p = readline(prompt);
+	PyOS_setsig(SIGINT, old_inthandler);
+
+    return p;
+}
+#endif /*defined(HAVE_RL_CALLBACK) && defined(HAVE_SELECT) */
+
+
+static char *
+call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
+{
+	size_t n;
+	char *p, *q;
+	int signal;
+
 	rl_event_hook = PyOS_InputHook;

 	if (sys_stdin != rl_instream || sys_stdout != rl_outstream) {
@ -697,16 +772,22 @@ call_readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
 #endif
 	}

-	p = readline(prompt);
-	PyOS_setsig(SIGINT, old_inthandler);
+	p = readline_until_enter_or_signal(prompt, &signal);
+	
+	/* we got an interrupt signal */
+	if(signal) {
+		return NULL;
+	}

-	/* We must return a buffer allocated with PyMem_Malloc. */
+	/* We got an EOF, return a empty string. */
 	if (p == NULL) {
 		p = PyMem_Malloc(1);
 		if (p != NULL)
 			*p = '\0';
 		return p;
 	}
+
+	/* we have a valid line */
 	n = strlen(p);
 	if (n > 0) {
 		char *line;
--- a/Parser/myreadline.c
+++ b/Parser/myreadline.c
@ -19,6 +19,14 @@
 extern char* vms__StdioReadline(FILE *sys_stdin, FILE *sys_stdout, char *prompt);
 #endif

+
+PyThreadState* _PyOS_ReadlineTState;
+
+#if WITH_THREAD
+#include "pythread.h"
+static PyThread_type_lock _PyOS_ReadlineLock = NULL;
+#endif
+
 int (*PyOS_InputHook)(void) = NULL;

 #ifdef RISCOS
@ -73,10 +81,13 @@ my_fgets(char *buf, int len, FILE *fp)
 		}
 #ifdef EINTR
 		if (errno == EINTR) {
-			if (PyOS_InterruptOccurred()) {
-				return 1; /* Interrupt */
+			int s;
+			PyEval_RestoreThread(_PyOS_ReadlineTState);
+			s = PyErr_CheckSignals();
+			PyThreadState_Swap(NULL);
+			if (s < 0) {
+				return 1;
 			}
-			continue;
 		}
 #endif
 		if (PyOS_InterruptOccurred()) {
@ -155,6 +166,13 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
 {
 	char *rv;

+	if (_PyOS_ReadlineTState == PyThreadState_GET()) {
+		PyErr_SetString(PyExc_RuntimeError,
+				"can't re-enter readline");
+		return NULL;
+	}
+	
+
 	if (PyOS_ReadlineFunctionPointer == NULL) {
 #ifdef __VMS
                PyOS_ReadlineFunctionPointer = vms__StdioReadline;
@ -162,8 +180,18 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
                PyOS_ReadlineFunctionPointer = PyOS_StdioReadline;
 #endif
 	}
+	
+#if WITH_THREAD
+	if (_PyOS_ReadlineLock == NULL) {
+		_PyOS_ReadlineLock = PyThread_allocate_lock();		
+	}
+#endif

+	_PyOS_ReadlineTState = PyThreadState_GET();
 	Py_BEGIN_ALLOW_THREADS
+#if WITH_THREAD
+	PyThread_acquire_lock(_PyOS_ReadlineLock, 1);
+#endif

        /* This is needed to handle the unlikely case that the
         * interpreter is in interactive mode *and* stdin/out are not
@ -176,5 +204,12 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, char *prompt)
                rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout,
                                                     prompt);
 	Py_END_ALLOW_THREADS
+
+#if WITH_THREAD
+	PyThread_release_lock(_PyOS_ReadlineLock);
+#endif
+
+	_PyOS_ReadlineTState = NULL;
+
 	return rv;
 }
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@ -1589,7 +1589,8 @@ builtin_raw_input(PyObject *self, PyObject *args)
                                  prompt);
 		Py_XDECREF(po);
 		if (s == NULL) {
-			PyErr_SetNone(PyExc_KeyboardInterrupt);
+			if (!PyErr_Occurred())
+				PyErr_SetNone(PyExc_KeyboardInterrupt);
 			return NULL;
 		}
 		if (*s == '\0') {
--- a/Python/ceval.c
+++ b/Python/ceval.c
@ -318,7 +318,7 @@ static volatile int things_to_do = 0;
 int
 Py_AddPendingCall(int (*func)(void *), void *arg)
 {
-	static int busy = 0;
+	static volatile int busy = 0;
 	int i, j;
 	/* XXX Begin critical section */
 	/* XXX If you want this to be safe against nested
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@ -1435,7 +1435,8 @@ err_input(perrdetail *err)
 		msg = "EOL while scanning single-quoted string";
 		break;
 	case E_INTR:
-		PyErr_SetNone(PyExc_KeyboardInterrupt);
+		if (!PyErr_Occurred())
+			PyErr_SetNone(PyExc_KeyboardInterrupt);
 		Py_XDECREF(v);
 		return;
 	case E_NOMEM:
--- a/Python/thread_pthread.h
+++ b/Python/thread_pthread.h
@ -119,7 +119,6 @@ PyThread_start_new_thread(void (*func)(void *), void *arg)
 {
 	pthread_t th;
 	int status;
- 	sigset_t oldmask, newmask;
 #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED)
 	pthread_attr_t attrs;
 #endif
@ -137,13 +136,6 @@ PyThread_start_new_thread(void (*func)(void *), void *arg)
        pthread_attr_setscope(&attrs, PTHREAD_SCOPE_SYSTEM);
 #endif

-	/* Mask all signals in the current thread before creating the new
-	 * thread.  This causes the new thread to start with all signals
-	 * blocked.
-	 */
-	sigfillset(&newmask);
-	SET_THREAD_SIGMASK(SIG_BLOCK, &newmask, &oldmask);
-
 	status = pthread_create(&th, 
 #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED)
 				 &attrs,
@ -154,9 +146,6 @@ PyThread_start_new_thread(void (*func)(void *), void *arg)
 				 (void *)arg
 				 );

-	/* Restore signal mask for original thread */
-	SET_THREAD_SIGMASK(SIG_SETMASK, &oldmask, NULL);
-
 #if defined(THREAD_STACK_SIZE) || defined(PTHREAD_SYSTEM_SCHED_SUPPORTED)
 	pthread_attr_destroy(&attrs);
 #endif
--- a/4491
+++ b/4491
--- a/configure.in
+++ b/configure.in
@ -2783,6 +2783,11 @@ then
  [Define this if you have flockfile(), getc_unlocked(), and funlockfile()])
 fi

+# check for readline 2.1
+AC_CHECK_LIB(readline, rl_callback_handler_install,
+	AC_DEFINE(HAVE_RL_CALLBACK, 1,
+        [Define if you have readline 2.1]), , -ltermcap)
+
 # check for readline 2.2
 AC_TRY_CPP([#include <readline/readline.h>],
 have_readline=yes, have_readline=no)
@ -2804,6 +2809,17 @@ AC_CHECK_LIB(readline, rl_completion_matches,
 	AC_DEFINE(HAVE_RL_COMPLETION_MATCHES, 1,
        [Define if you have readline 4.2]), , -ltermcap)

+# also in readline 4.2
+AC_TRY_CPP([#include <readline/readline.h>],
+have_readline=yes, have_readline=no)
+if test $have_readline = yes
+then
+  AC_EGREP_HEADER([extern int rl_catch_signals;],
+  [readline/readline.h],
+  AC_DEFINE(HAVE_RL_CATCH_SIGNAL, 1,
+  [Define if you can turn off readline's signal handling.]), )
+fi
+
 AC_MSG_CHECKING(for broken nice())
 AC_CACHE_VAL(ac_cv_broken_nice, [
 AC_TRY_RUN([
--- a/pyconfig.h.in
+++ b/pyconfig.h.in
@ -359,6 +359,12 @@
 /* Define to 1 if you have the `realpath' function. */
 #undef HAVE_REALPATH

+/* Define if you have readline 2.1 */
+#undef HAVE_RL_CALLBACK
+
+/* Define if you can turn off readline's signal handling. */
+#undef HAVE_RL_CATCH_SIGNAL
+
 /* Define if you have readline 2.2 */
 #undef HAVE_RL_COMPLETION_APPEND_CHARACTER