From: Daniel Colascione <dancol@dancol.org>
To: emacs-devel@gnu.org, dancol@dancol.org
Subject: [PATCH] Really prevent quitting in sensitive contexts
Date: Wed, 11 Mar 2020 19:05:54 -0700 [thread overview]
Message-ID: <20200312020554.194607-1-dancol@dancol.org> (raw)
Emacs has traditionally inhibited quitting in process filters and a
few other contexts to prevent stray user-typed C-g keystrokes from
interfering with "background" tasks like recording process output.
This quit inhibition is accomplished by binding inhibit-quit to t
around these sensitive regions. The trouble is that code like
with-local-quit can temporarily bind inhibit-quit to nil inside code
running inside a process filter and thereby make "background" against
work sensitive to stray C-g keystrokes.
with-local-quit is actually documented to allow this sort of
foreground quitting, but since the interactive C-g race is
unavoidable, we should deprecate use of with-local-quit for this
purpose. For compatibility, we can't remove it entirely.
This change adds a new inhibit-quit-override flag. When non-nil,
inhibit-quit-override makes Emacs behave as if inhibit-quit were t
regardless of inhibit-quit's actual value. Emacs sets
inhibit-quit-override inside process filters and other critical
regions and ensures that with-local-quit and other binders of
inhibit-quit don't accidentally allow C-g to influence these contexts.
* src/process.c (wait_reading_process_output): Check
Vinhibit_quit_override as well as Vinhibit_quit.
(read_and_dispose_of_process_output, exec_sentinel): Bind
Vinhibit_quit_override instead of Vinhibit_quit.
* src/lisp.h (QUITP): check Vinhibit_quit_override as well as
Vinhibit_quit.
* src/eval.c (maybe_quit): Use QUITP instead of open-coded check.
(Qinhibit_quit_override, Vinhibit_quit_override): Define.
* src/ccl.c (ccl_driver): Use QUITP instead of open-coded check.
* src/keyboard.c
(menu_bar_items,tab_bar_items,tool_bar_items,safe_run_hooks)
(timer_check_2): Set Vinhibit_quit_override instead of
Vinhibit_quit.
(handle_interrupt): Set Vinhibit_quit_override to nil
after ten quits.
---
etc/NEWS | 5 +++++
src/ccl.c | 2 +-
src/eval.c | 11 ++++++++++-
src/keyboard.c | 32 +++++++++++++++++---------------
src/lisp.h | 4 +++-
src/process.c | 8 +++++---
src/xdisp.c | 10 +++++-----
7 files changed, 46 insertions(+), 26 deletions(-)
diff --git a/etc/NEWS b/etc/NEWS
index fcdf6dbe24..6181ef2c18 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -193,6 +193,11 @@ are 'eq'. To compare contents, use 'compare-window-configurations'
instead. This change helps fix a bug in 'sxhash-equal', which returned
incorrect hashes for window configurations and some other objects.
+** When the new 'inhibit-quit-override' variable is non-nil, Emacs
+behaves as if 'inhibit-quit' were t. This change prevents
+with-local-quit from accidentally allowing quits inside process
+filters, redisplay callbacks, and other special contexts.
+
---
** The obsolete function 'thread-alive-p' has been removed.
diff --git a/src/ccl.c b/src/ccl.c
index ac44dc1f60..a495d5a4ca 100644
--- a/src/ccl.c
+++ b/src/ccl.c
@@ -893,7 +893,7 @@ ccl_driver (struct ccl_program *ccl, int *source, int *destination, int src_size
ccl_backtrace_table[ccl_backtrace_idx] = 0;
#endif
- if (!NILP (Vquit_flag) && NILP (Vinhibit_quit))
+ if (QUITP)
{
/* We can't just signal Qquit, instead break the loop as if
the whole data is processed. Don't reset Vquit_flag, it
diff --git a/src/eval.c b/src/eval.c
index 4559a0e1f6..bbfb3579d2 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1539,7 +1539,7 @@ process_quit_flag (void)
void
maybe_quit (void)
{
- if (!NILP (Vquit_flag) && NILP (Vinhibit_quit))
+ if (QUITP)
process_quit_flag ();
else if (pending_signals)
process_pending_signals ();
@@ -4062,8 +4062,17 @@ syms_of_eval (void)
before making `inhibit-quit' nil. */);
Vinhibit_quit = Qnil;
+ DEFVAR_LISP ("inhibit-quit-override", Vinhibit_quit_override,
+ doc: /* If t, consider inhibit-quit t even if it's set
+to nil. Emacs uses this flag to prevent with-local-quit from
+temporarily allowing quits in places where we really don't want to
+allow quits at all, e.g., while running filters. Never locally bind
+this symbol to nil! */);
+ Vinhibit_quit_override = Qnil;
+
DEFSYM (Qsetq, "setq");
DEFSYM (Qinhibit_quit, "inhibit-quit");
+ DEFSYM (Qinhibit_quit_override, "inhibit-quit-override");
DEFSYM (Qautoload, "autoload");
DEFSYM (Qinhibit_debugger, "inhibit-debugger");
DEFSYM (Qmacro, "macro");
diff --git a/src/keyboard.c b/src/keyboard.c
index 9ce168c6dd..ab2890b117 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -1834,7 +1834,7 @@ safe_run_hooks (Lisp_Object hook)
{
ptrdiff_t count = SPECPDL_INDEX ();
- specbind (Qinhibit_quit, Qt);
+ specbind (Qinhibit_quit_override, Qt);
run_hook_with_args (2, ((Lisp_Object []) {hook, hook}), safe_run_hook_funcall);
unbind_to (count, Qnil);
}
@@ -4337,7 +4337,7 @@ timer_check_2 (Lisp_Object timers, Lisp_Object idle_timers)
code fails to reschedule it right. */
ASET (chosen_timer, 0, Qt);
- specbind (Qinhibit_quit, Qt);
+ specbind (Qinhibit_quit_override, Qt);
call1 (Qtimer_event_handler, chosen_timer);
Vdeactivate_mark = old_deactivate_mark;
@@ -4382,8 +4382,8 @@ timer_check (void)
struct timespec nexttime;
Lisp_Object timers, idle_timers;
- Lisp_Object tem = Vinhibit_quit;
- Vinhibit_quit = Qt;
+ Lisp_Object tem = Vinhibit_quit_override;
+ Vinhibit_quit_override = Qt;
/* We use copies of the timers' lists to allow a timer to add itself
again, without locking up Emacs if the newly added timer is
@@ -4397,7 +4397,7 @@ timer_check (void)
else
idle_timers = Qnil;
- Vinhibit_quit = tem;
+ Vinhibit_quit_override = tem;
do
{
@@ -7417,8 +7417,8 @@ menu_bar_items (Lisp_Object old)
quitting while building the menus.
We do this instead of specbind because (1) errors will clear it anyway
and (2) this avoids risk of specpdl overflow. */
- oquit = Vinhibit_quit;
- Vinhibit_quit = Qt;
+ oquit = Vinhibit_quit_override;
+ Vinhibit_quit_override = Qt;
if (!NILP (old))
menu_bar_items_vector = old;
@@ -7528,7 +7528,7 @@ menu_bar_items (Lisp_Object old)
menu_bar_items_index = i;
}
- Vinhibit_quit = oquit;
+ Vinhibit_quit_override = oquit;
SAFE_FREE ();
return menu_bar_items_vector;
}
@@ -7978,8 +7978,8 @@ tab_bar_items (Lisp_Object reuse, int *nitems)
quitting while building the menus. We do this instead of
specbind because (1) errors will clear it anyway and (2) this
avoids risk of specpdl overflow. */
- oquit = Vinhibit_quit;
- Vinhibit_quit = Qt;
+ oquit = Vinhibit_quit_override;
+ Vinhibit_quit_override = Qt;
/* Initialize tab_bar_items_vector and protect it from GC. */
init_tab_bar_items (reuse);
@@ -8037,7 +8037,7 @@ tab_bar_items (Lisp_Object reuse, int *nitems)
map_keymap (keymap, process_tab_bar_item, Qnil, NULL, 1);
}
- Vinhibit_quit = oquit;
+ Vinhibit_quit_override = oquit;
*nitems = ntab_bar_items / TAB_BAR_ITEM_NSLOTS;
SAFE_FREE ();
return tab_bar_items_vector;
@@ -8362,8 +8362,8 @@ tool_bar_items (Lisp_Object reuse, int *nitems)
quitting while building the menus. We do this instead of
specbind because (1) errors will clear it anyway and (2) this
avoids risk of specpdl overflow. */
- oquit = Vinhibit_quit;
- Vinhibit_quit = Qt;
+ oquit = Vinhibit_quit_override;
+ Vinhibit_quit_override = Qt;
/* Initialize tool_bar_items_vector and protect it from GC. */
init_tool_bar_items (reuse);
@@ -8421,7 +8421,7 @@ tool_bar_items (Lisp_Object reuse, int *nitems)
map_keymap (keymap, process_tool_bar_item, Qnil, NULL, 1);
}
- Vinhibit_quit = oquit;
+ Vinhibit_quit_override = oquit;
*nitems = ntool_bar_items / TOOL_BAR_ITEM_NSLOTS;
SAFE_FREE ();
return tool_bar_items_vector;
@@ -10916,7 +10916,9 @@ handle_interrupt (bool in_signal_handler)
int count = NILP (Vquit_flag) ? 1 : force_quit_count + 1;
force_quit_count = count;
if (count == 3)
- Vinhibit_quit = Qnil;
+ Vinhibit_quit = Qnil;
+ if (count == 10)
+ Vinhibit_quit_override = Qnil;
Vquit_flag = Qt;
}
diff --git a/src/lisp.h b/src/lisp.h
index a379977d35..2d7cb85fab 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3345,7 +3345,9 @@ SPECPDL_INDEX (void)
/* True if ought to quit now. */
-#define QUITP (!NILP (Vquit_flag) && NILP (Vinhibit_quit))
+#define QUITP (!NILP (Vquit_flag) \
+ && NILP (Vinhibit_quit) \
+ && NILP (Vinhibit_quit_override))
/* Process a quit rarely, based on a counter COUNT, for efficiency.
"Rarely" means once per USHRT_MAX + 1 times; this is somewhat
diff --git a/src/process.c b/src/process.c
index 91d426103d..f280588f51 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5214,7 +5214,9 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
FD_ZERO (&Available);
FD_ZERO (&Writeok);
- if (time_limit == 0 && nsecs == 0 && wait_proc && !NILP (Vinhibit_quit)
+ if (time_limit == 0 && nsecs == 0 && wait_proc
+ && !NILP (Vinhibit_quit)
+ && !NILP (Vinhibit_quit_override)
&& !(CONSP (wait_proc->status)
&& EQ (XCAR (wait_proc->status), Qexit)))
message1 ("Blocking call to accept-process-output with quit inhibited!!");
@@ -6145,7 +6147,7 @@ read_and_dispose_of_process_output (struct Lisp_Process *p, char *chars,
/* We inhibit quit here instead of just catching it so that
hitting ^G when a filter happens to be running won't screw
it up. */
- specbind (Qinhibit_quit, Qt);
+ specbind (Qinhibit_quit_override, Qt);
specbind (Qlast_nonmenu_event, Qt);
/* In case we get recursively called,
@@ -7364,7 +7366,7 @@ exec_sentinel (Lisp_Object proc, Lisp_Object reason)
sentinel = p->sentinel;
/* Inhibit quit so that random quits don't screw up a running filter. */
- specbind (Qinhibit_quit, Qt);
+ specbind (Qinhibit_quit_override, Qt);
specbind (Qlast_nonmenu_event, Qt); /* Why? --Stef */
/* In case we get recursively called,
diff --git a/src/xdisp.c b/src/xdisp.c
index c2aa314c1a..b3a7512dcc 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2801,7 +2801,7 @@ safe__call (bool inhibit_quit, ptrdiff_t nargs, Lisp_Object func, va_list ap)
specbind (Qinhibit_redisplay, Qt);
if (inhibit_quit)
- specbind (Qinhibit_quit, Qt);
+ specbind (Qinhibit_quit_override, Qt);
/* Use Qt to ensure debugger does not run,
so there is no possibility of wanting to redisplay. */
val = internal_condition_case_n (Ffuncall, nargs, args, Qt,
@@ -27259,13 +27259,13 @@ dump_glyph_string (struct glyph_string *s)
section. If any of the code run by callers of ALLOCATE_HDC happens
to call Lisp (might be possible due to all the hooks lying around),
we must prevent it from quitting. */
-# define ALLOCATE_HDC(hdc, f) \
- Lisp_Object prev_quit = Vinhibit_quit; \
- Vinhibit_quit = Qt; \
+# define ALLOCATE_HDC(hdc, f) \
+ Lisp_Object prev_quit = Vinhibit_quit_override; \
+ Vinhibit_quit_override = Qt; \
HDC hdc = get_frame_dc ((f))
# define RELEASE_HDC(hdc, f) \
release_frame_dc ((f), (hdc)); \
- Vinhibit_quit = prev_quit
+ Vinhibit_quit_override = prev_quit
#else
# define ALLOCATE_HDC(hdc, f)
# define RELEASE_HDC(hdc, f)
--
2.25.1.481.gfbce0eb801-goog
next reply other threads:[~2020-03-12 2:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-12 2:05 Daniel Colascione [this message]
2020-03-12 2:20 ` [PATCH] Really prevent quitting in sensitive contexts Stefan Monnier
2020-03-12 2:32 ` Daniel Colascione
2020-03-12 10:07 ` Michael Albinus
2020-03-12 10:21 ` Robert Pluim
2020-03-12 10:33 ` Michael Albinus
2020-03-13 10:00 ` Michael Albinus
2020-04-14 6:56 ` Daniel Colascione
2020-04-14 8:40 ` Michael Albinus
2020-03-12 3:29 ` Óscar Fuentes
2020-03-12 7:21 ` Eli Zaretskii
2020-03-12 13:31 ` Richard Copley
2020-03-12 17:36 ` Drew Adams
2020-03-12 22:06 ` Stefan Monnier
2020-03-12 23:14 ` Richard Copley
2020-03-12 23:35 ` Stefan Monnier
2020-03-12 23:56 ` Richard Copley
2020-03-14 15:31 ` Drew Adams
2020-03-14 18:21 ` Stefan Monnier
2020-03-14 18:36 ` Drew Adams
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200312020554.194607-1-dancol@dancol.org \
--to=dancol@dancol.org \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).