diff options
| -rw-r--r-- | configure.ac | 2 | ||||
| -rw-r--r-- | src/interface-curses/interface.c | 59 | ||||
| -rw-r--r-- | src/interface-gtk/interface.c | 47 | ||||
| -rw-r--r-- | src/main.c | 35 | ||||
| -rw-r--r-- | src/sciteco.h | 5 | ||||
| -rw-r--r-- | src/spawn.c | 67 |
6 files changed, 126 insertions, 89 deletions
diff --git a/configure.ac b/configure.ac index 1dec488..b8e6e38 100644 --- a/configure.ac +++ b/configure.ac @@ -170,7 +170,7 @@ AC_CHECK_FUNCS([memset setlocale strchr strrchr fstat sscanf], , [ # glib defines G_OS_UNIX instead... case $host in *-*-linux* | *-*-*bsd* | *-*-darwin* | *-*-cygwin* | *-*-haiku*) - AC_CHECK_FUNCS([realpath fchown dup dup2 getpid open read mmap], , [ + AC_CHECK_FUNCS([realpath fchown dup dup2 getpid open read kill mmap], , [ AC_MSG_ERROR([Missing libc function]) ]) ;; diff --git a/src/interface-curses/interface.c b/src/interface-curses/interface.c index 25311c5..e056333 100644 --- a/src/interface-curses/interface.c +++ b/src/interface-curses/interface.c @@ -103,7 +103,7 @@ #define CURSES_TTY #endif -#if defined(PDCURSES_WINCON) || defined(NCURSES_WIN32) +#ifdef G_OS_WIN32 /** * This handler is the Windows-analogue of a signal @@ -119,7 +119,7 @@ teco_console_ctrl_handler(DWORD type) { switch (type) { case CTRL_C_EVENT: - teco_sigint_occurred = TRUE; + teco_interrupted = TRUE; return TRUE; } @@ -397,6 +397,16 @@ teco_interface_init(void) #ifndef CURSES_TTY teco_interface_init_clipboard(); #endif + + /* + * The default SIGINT signal handler seems to partially work + * as the console control handler. + * However, a second CTRL+C event (or raise(SIGINT)) would + * terminate the process. + */ +#ifdef G_OS_WIN32 + SetConsoleCtrlHandler(teco_console_ctrl_handler, TRUE); +#endif } GOptionGroup * @@ -675,14 +685,11 @@ teco_interface_init_interactive(GError **error) teco_interface_init_screen(); /* - * We must register this handler for - * asynchronous interruptions via CTRL+C - * reliably. The signal handler we already - * have won't do. - * Doing this here ensures that we have a higher - * precedence than the handler installed by PDCursesMod. + * We always have a CTRL handler on Windows, but doing it + * here again, ensures that we have a higher precedence + * than the one installed by PDCurses. */ -#if defined(PDCURSES_WINCON) || defined(NCURSES_WIN32) +#ifdef G_OS_WIN32 SetConsoleCtrlHandler(teco_console_ctrl_handler, TRUE); #endif @@ -1465,7 +1472,7 @@ teco_interface_popup_clear(void) gboolean teco_interface_is_interrupted(void) { - return teco_sigint_occurred != FALSE; + return teco_interrupted != FALSE; } #else /* !CURSES_TTY && !PDCURSES_WINCON && !NCURSES_WIN32 */ @@ -1473,27 +1480,29 @@ teco_interface_is_interrupted(void) /* * This function is called repeatedly, so we can poll the keyboard input queue, * filtering out CTRL+C. - * This is naturally very inefficient. * It's currently necessary as a fallback e.g. for PDCURSES_GUI or XCurses. + * + * NOTE: Theoretically, this can be optimized by doing wgetch() only every X + * microseconds like on Gtk+. + * But this turned out to slow things down, at least on PDCurses/WinGUI. */ gboolean teco_interface_is_interrupted(void) { - if (teco_interface.cmdline_window) { /* interactive mode */ - gint key; - - /* - * NOTE: getch() is configured to be nonblocking. - */ - while ((key = wgetch(teco_interface.cmdline_window)) != ERR) { - if (G_UNLIKELY(key == TECO_CTL_KEY('C'))) - return TRUE; - g_queue_push_tail(teco_interface.input_queue, - GINT_TO_POINTER(key)); - } + if (!teco_interface.cmdline_window) + /* batch mode */ + return teco_interrupted != FALSE; + + /* NOTE: getch() is configured to be nonblocking. */ + gint key; + while ((key = wgetch(teco_interface.cmdline_window)) != ERR) { + if (G_UNLIKELY(key == TECO_CTL_KEY('C'))) + return TRUE; + g_queue_push_tail(teco_interface.input_queue, + GINT_TO_POINTER(key)); } - return teco_sigint_occurred != FALSE; + return teco_interrupted != FALSE; } #endif @@ -1528,7 +1537,7 @@ teco_interface_blocking_getch(void) gint key = wgetch(teco_interface.cmdline_window); teco_memory_start_limiting(); /* allow asynchronous interruptions on <CTRL/C> */ - teco_sigint_occurred = FALSE; + teco_interrupted = FALSE; nodelay(teco_interface.cmdline_window, TRUE); #if defined(CURSES_TTY) || defined(PDCURSES_WINCON) || defined(NCURSES_WIN32) noraw(); /* FIXME: necessary because of NCURSES_WIN32 bug */ diff --git a/src/interface-gtk/interface.c b/src/interface-gtk/interface.c index d22009b..6301a71 100644 --- a/src/interface-gtk/interface.c +++ b/src/interface-gtk/interface.c @@ -70,6 +70,12 @@ static gboolean teco_interface_window_delete_cb(GtkWidget *widget, GdkEventAny * gpointer user_data); static gboolean teco_interface_sigterm_handler(gpointer user_data) G_GNUC_UNUSED; +/** + * Interval between polling for keypresses. + * In other words, this is the maximum latency to detect CTRL+C interruptions. + */ +#define TECO_POLL_INTERVAL 100000 /* microseconds */ + #define UNNAMED_FILE "(Unnamed)" #define USER_CSS_FILE ".teco_css" @@ -719,22 +725,27 @@ teco_interface_popup_clear(void) * system call overhead. * But the GDK lock that would be necessary for synchronization * has been deprecated. - * - * @todo It would be great to have platform-specific optimizations, - * so we can detect interruptions without having to drive the Glib - * event loop (e.g. using libX11 or Win32 APIs). - * There already is a keyboard hook for Win32 in interface-curses. - * On the downside, such solutions will probably freeze the window - * while SciTECO is busy. However we currently freeze the window - * anyway while being busy to avoid flickering. */ gboolean teco_interface_is_interrupted(void) { - if (gtk_main_level() > 0) - gtk_main_iteration_do(FALSE); + if (!gtk_main_level()) + /* batch mode */ + return teco_interrupted != FALSE; - return teco_sigint_occurred != FALSE; + /* + * By polling only every TECO_POLL_INTERVAL microseconds + * we save 75-90% of runtime. + */ + static guint64 last_poll_ts = 0; + guint64 now_ts = g_get_monotonic_time(); + + if (G_LIKELY(last_poll_ts+TECO_POLL_INTERVAL > now_ts)) + return teco_interrupted != FALSE; + last_poll_ts = now_ts; + + gtk_main_iteration_do(FALSE); + return teco_interrupted != FALSE; } static void @@ -1172,12 +1183,10 @@ teco_interface_key_pressed_cb(GtkWidget *widget, GdkEventKey *event, gpointer us gdk_keyval_to_upper(event->keyval) == GDK_KEY_C) /* * Handle asynchronous interruptions if CTRL+C is pressed. - * This will usually send SIGINT to the entire process - * group and set `teco_sigint_occurred`. * If the execution thread is currently blocking, * the key is delivered like an ordinary key press. */ - teco_interrupt(); + teco_interrupted = TRUE; else g_queue_push_tail(teco_interface.event_queue, gdk_event_copy((GdkEvent *)event)); @@ -1209,9 +1218,9 @@ teco_interface_key_pressed_cb(GtkWidget *widget, GdkEventKey *event, gpointer us */ gdk_window_freeze_updates(top_window); - teco_sigint_occurred = FALSE; + teco_interrupted = FALSE; teco_interface_handle_key_press(&event->key, &error); - teco_sigint_occurred = FALSE; + teco_interrupted = FALSE; gdk_window_thaw_updates(top_window); @@ -1256,12 +1265,6 @@ static gboolean teco_interface_sigterm_handler(gpointer user_data) { /* - * Since this handler replaces the default signal handler, - * we also have to make sure it interrupts. - */ - teco_interrupt(); - - /* * Similar to window deletion - emulate "close" key press. */ g_autoptr(GdkEvent) close_event = gdk_event_new(GDK_KEY_PRESS); @@ -52,32 +52,13 @@ teco_int_t teco_ed = TECO_ED_AUTOEOL; -volatile sig_atomic_t teco_sigint_occurred = FALSE; - -#ifdef G_OS_UNIX - -void -teco_interrupt(void) -{ - /* - * This sends SIGINT to the entire process group, - * which makes sure that subprocesses are signalled, - * even when called from the wrong thread. - */ - if (kill(0, SIGINT)) - teco_sigint_occurred = TRUE; -} - -#else /* !G_OS_UNIX */ - -void -teco_interrupt(void) -{ - if (raise(SIGINT)) - teco_sigint_occurred = TRUE; -} - -#endif +/** + * Whether there was an asyncronous interruption (usually after pressing CTRL+C). + * However you should always use teco_interface_is_interrupted(), + * to check for interruptions because of its side effects. + * This variable is safe to set to TRUE from signal handlers and threads. + */ +volatile sig_atomic_t teco_interrupted = FALSE; /* * FIXME: Move this into file-utils.c? @@ -309,7 +290,7 @@ teco_initialize_environment(const gchar *program) static void teco_sigint_handler(int signal) { - teco_sigint_occurred = TRUE; + teco_interrupted = TRUE; } int diff --git a/src/sciteco.h b/src/sciteco.h index 4f3d88a..87bd973 100644 --- a/src/sciteco.h +++ b/src/sciteco.h @@ -85,10 +85,7 @@ enum { extern teco_int_t teco_ed; /* in main.c */ -extern volatile sig_atomic_t teco_sigint_occurred; - -/* in main.c */ -void teco_interrupt(void); +extern volatile sig_atomic_t teco_interrupted; /* * Allows automatic cleanup of FILE pointers. diff --git a/src/spawn.c b/src/spawn.c index 416c433..d79259e 100644 --- a/src/spawn.c +++ b/src/spawn.c @@ -19,8 +19,18 @@ #include "config.h" #endif +#include <signal.h> + #include <glib.h> +#include <gmodule.h> +#include <glib/gprintf.h> + +#ifdef HAVE_WINDOWS_H +#define WIN32_LEAN_AND_MEAN +#include <windows.h> +#endif + #include "sciteco.h" #include "interface.h" #include "undo.h" @@ -60,8 +70,10 @@ static struct { GMainContext *mainctx; GMainLoop *mainloop; GSource *idle_src; + GPid pid; GSource *child_src; GSource *stdin_src, *stdout_src; + gboolean interrupted; teco_int_t from, to; teco_int_t start; @@ -220,10 +232,13 @@ teco_state_execute_done(teco_machine_main_t *ctx, const teco_string_t *str, GErr /* * NOTE: With G_SPAWN_LEAVE_DESCRIPTORS_OPEN and without G_SPAWN_SEARCH_PATH_FROM_ENVP, * Glib offers an "optimized codepath" on UNIX. - * G_SPAWN_SEARCH_PATH_FROM_ENVP does not appear to work on Windows, anyway. */ static const GSpawnFlags flags = G_SPAWN_DO_NOT_REAP_CHILD | +#ifdef G_OS_UNIX G_SPAWN_SEARCH_PATH | G_SPAWN_LEAVE_DESCRIPTORS_OPEN | +#else + G_SPAWN_SEARCH_PATH_FROM_ENVP | +#endif G_SPAWN_STDERR_TO_DEV_NULL; if (ctx->mode > TECO_MODE_NORMAL) @@ -257,17 +272,17 @@ teco_state_execute_done(teco_machine_main_t *ctx, const teco_string_t *str, GErr if (!envp) goto gerror; - GPid pid; gint stdin_fd, stdout_fd; - if (!g_spawn_async_with_pipes(NULL, argv, envp, flags, NULL, NULL, &pid, + if (!g_spawn_async_with_pipes(NULL, argv, envp, flags, NULL, NULL, &teco_spawn_ctx.pid, &stdin_fd, &stdout_fd, NULL, error)) goto gerror; - teco_spawn_ctx.child_src = g_child_watch_source_new(pid); + teco_spawn_ctx.child_src = g_child_watch_source_new(teco_spawn_ctx.pid); g_source_set_callback(teco_spawn_ctx.child_src, (GSourceFunc)teco_spawn_child_watch_cb, NULL, NULL); g_source_attach(teco_spawn_ctx.child_src, teco_spawn_ctx.mainctx); + teco_spawn_ctx.interrupted = FALSE; #ifdef G_OS_WIN32 stdin_chan = g_io_channel_win32_new_fd(stdin_fd); @@ -343,11 +358,10 @@ teco_state_execute_done(teco_machine_main_t *ctx, const teco_string_t *str, GErr g_source_unref(teco_spawn_ctx.stdout_src); g_source_unref(teco_spawn_ctx.child_src); - g_spawn_close_pid(pid); + g_spawn_close_pid(teco_spawn_ctx.pid); /* - * NOTE: This includes interruptions due to teco_interrupt() - * following CTRL+C. + * NOTE: This includes interruptions following CTRL+C. * But they are reported as G_SPAWN_ERROR_FAILED and hard to filter out. */ if (teco_spawn_ctx.error) { @@ -487,6 +501,9 @@ gboolean teco_state_execute_process_edit_cmd(teco_machine_main_t *ctx, teco_mach * the <command> completes, which may result in editor hangs. * You may however interrupt the spawned process by sending * the \fBSIGINT\fP signal to \*(ST, e.g. by pressing CTRL+C. + * The first time, this will try to kill the spawned process + * gracefully. + * The second time you press CTRL+C, it will hard kill the process. * * In interactive mode, \*(ST performs TAB-completion * of filenames in the <command> string parameter but @@ -687,9 +704,39 @@ error: static gboolean teco_spawn_idle_cb(gpointer user_data) { - if (G_UNLIKELY(teco_interface_is_interrupted())) - /* Just in case this didn't yet happen. Should kill the child process. */ - teco_interrupt(); + if (G_LIKELY(!teco_interface_is_interrupted())) + return G_SOURCE_CONTINUE; + teco_interrupted = FALSE; + +#ifdef G_OS_WIN32 + /* + * FIXME: We could actually try to gracefully terminate the process first + * using GenerateConsoleCtrlEvent(CTRL_C_EVENT). + * However, it's hard to find the correct process group id. + * We can pass 0, but this will fire our own control handler again, + * resulting in a hard kill via TerminateProcess() anyway. + * Masking the control handler is also not viable because it's a race + * condition. All the workarounds would be very hacky. + * + * FIXME: TerminateProcess() does not work on processes that ignore their + * control handler after calling SetConsoleCtrlHandler(NULL, TRUE), + * even though it correctly sets its return code. + * SciTECO will currently simply hang on these processes - but you can + * kill the child process via the task manager. + */ + TerminateProcess(teco_spawn_ctx.pid, 1); +#elif defined(G_OS_UNIX) + /* + * The first CTRL+C will try to gracefully terminate the process. + */ + if (teco_spawn_ctx.interrupted || kill(teco_spawn_ctx.pid, SIGINT); + kill(teco_spawn_ctx.pid, SIGKILL); +#else + /* This may signal unrelated processes as well. */ + raise(SIGINT); +#endif + teco_spawn_ctx.interrupted = TRUE; + return G_SOURCE_CONTINUE; } |