unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Really prevent quitting in sensitive contexts
@ 2020-03-12  2:05 Daniel Colascione
  2020-03-12  2:20 ` Stefan Monnier
  0 siblings, 1 reply; 20+ messages in thread
From: Daniel Colascione @ 2020-03-12  2:05 UTC (permalink / raw)
  To: emacs-devel, dancol

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




^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12  2:05 [PATCH] Really prevent quitting in sensitive contexts Daniel Colascione
@ 2020-03-12  2:20 ` Stefan Monnier
  2020-03-12  2:32   ` Daniel Colascione
                     ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Stefan Monnier @ 2020-03-12  2:20 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/x-markdown; coding=UTF-8, Size: 837 bytes --]

> with-local-quit is actually documented to allow this sort of
> foreground quitting,

AFAIK it's not just documented to allow that, but it's designed
specifically for that.

> but since the interactive C-g race is unavoidable, we should deprecate
> use of with-local-quit for this purpose.

I tend to agree.

> This change adds a new inhibit-quit-override flag.

Sounds like an arm's race: then we'll have to add
a with-local-quit-override-override, etc...

> +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.

I'm not really familiar with those accidental cases.
Maybe they can be fixed more directly by not using with-local-quit?

Are there bug#NNN where these are discussed?


        Stefan




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12  2:20 ` Stefan Monnier
@ 2020-03-12  2:32   ` Daniel Colascione
  2020-03-12 10:07     ` Michael Albinus
  2020-03-12  3:29   ` Óscar Fuentes
  2020-03-12 13:31   ` Richard Copley
  2 siblings, 1 reply; 20+ messages in thread
From: Daniel Colascione @ 2020-03-12  2:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 3/11/20 7:20 PM, Stefan Monnier wrote:
>> with-local-quit is actually documented to allow this sort of
>> foreground quitting,
> 
> AFAIK it's not just documented to allow that, but it's designed
> specifically for that.

Right, but I can imagine it being useful more generally in some code 
that wants to be atomic against quit, but with exceptions. I don't know 
of any such code though.

>> but since the interactive C-g race is unavoidable, we should deprecate
>> use of with-local-quit for this purpose.
> 
> I tend to agree.
> 
>> This change adds a new inhibit-quit-override flag.
> 
> Sounds like an arm's race: then we'll have to add
> a with-local-quit-override-override, etc...

I considered making with-local-quit-override inaccessible from Lisp.

>> +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.
> 
> I'm not really familiar with those accidental cases.
> Maybe they can be fixed more directly by not using with-local-quit?

with-local-quit has been there for ten years. Can we just delete it?

> 
> Are there bug#NNN where these are discussed?

No. The motiviation is Tramp inflooping after a quit because it uses 
with-local-quit inside tramp-accept-process-output, which causes 
tramp-wait-for-regexp to retry infinitely and quickly.

I found *that* behavior trying to figure out why M-x compile over Tramp 
was hanging not only Emacs, but also the SSH connection more generally. 
I *think* it has something to do with compilation-filter calling 
file-truename, which invokes Tramp's handler, which wants to talk over 
the same SSH connection that's spewing compilation messages, deadlocking 
something somewhere.

We probably should make compile not filename lookup directly in the 
process filter.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12  2:20 ` Stefan Monnier
  2020-03-12  2:32   ` Daniel Colascione
@ 2020-03-12  3:29   ` Óscar Fuentes
  2020-03-12  7:21     ` Eli Zaretskii
  2020-03-12 13:31   ` Richard Copley
  2 siblings, 1 reply; 20+ messages in thread
From: Óscar Fuentes @ 2020-03-12  3:29 UTC (permalink / raw)
  To: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +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.
>
> I'm not really familiar with those accidental cases.
> Maybe they can be fixed more directly by not using with-local-quit?
>
> Are there bug#NNN where these are discussed?

bug#31692 dealed with missing key events and while-no-input /
with-local-quit were discussed.




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12  3:29   ` Óscar Fuentes
@ 2020-03-12  7:21     ` Eli Zaretskii
  0 siblings, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2020-03-12  7:21 UTC (permalink / raw)
  To: emacs-devel, Óscar Fuentes

On March 12, 2020 5:29:33 AM GMT+02:00, "Óscar Fuentes" <ofv@wanadoo.es> wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > Are there bug#NNN where these are discussed?
> 
> bug#31692 dealed with missing key events and while-no-input /
> with-local-quit were discussed.

AFAICT, that bug discussion is about while-no-input, and thus not really related to the issue at hand here.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12  2:32   ` Daniel Colascione
@ 2020-03-12 10:07     ` Michael Albinus
  2020-03-12 10:21       ` Robert Pluim
  2020-04-14  6:56       ` Daniel Colascione
  0 siblings, 2 replies; 20+ messages in thread
From: Michael Albinus @ 2020-03-12 10:07 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: Stefan Monnier, emacs-devel

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

Daniel Colascione <dancol@dancol.org> writes:

>> Are there bug#NNN where these are discussed?
>
> No. The motiviation is Tramp inflooping after a quit because it uses
> with-local-quit inside tramp-accept-process-output, which causes
> tramp-wait-for-regexp to retry infinitely and quickly.
>
> I found *that* behavior trying to figure out why M-x compile over
> Tramp was hanging not only Emacs, but also the SSH connection more
> generally. I *think* it has something to do with compilation-filter
> calling file-truename, which invokes Tramp's handler, which wants to
> talk over the same SSH connection that's spewing compilation messages,
> deadlocking something somewhere.

Please write a bug report, it should be solved in Tramp.

with-local-quit is used in tramp-accept-process-output by
intention. There were reports that Tramp hung, and couldn't be quit ...

Would the appended patch help (completely untested)?

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1342 bytes --]

diff --git a/lisp/tramp.el b/lisp/tramp.el
index 82ac693d..af2bc4b2 100644
--- a/lisp/tramp.el
+++ b/lisp/tramp.el
@@ -4227,18 +4227,20 @@ performed successfully.  Any other value means an error."
 (defun tramp-accept-process-output (proc &optional timeout)
   "Like `accept-process-output' for Tramp processes.
 This is needed in order to hide `last-coding-system-used', which is set
-for process communication also."
+for process communication also.
+If the user quits via `C-g', it is propagated up to `tramp-file-name-handler'."
   (with-current-buffer (process-buffer proc)
     (let ((inhibit-read-only t)
 	  last-coding-system-used
 	  result)
-      ;; JUST-THIS-ONE is set due to Bug#12145.
-      (tramp-message
-       proc 10 "%s %s %s %s\n%s"
-       proc timeout (process-status proc)
-       (with-local-quit
-	 (setq result (accept-process-output proc timeout nil t)))
-       (buffer-string))
+      ;; JUST-THIS-ONE is set due to Bug#12145.  `with-local-quit'
+      ;; returns t in order to report success.
+      (if (with-local-quit
+	    (setq result (accept-process-output proc timeout nil t)) t)
+	  (tramp-message
+	   proc 10 "%s %s %s %s\n%s"
+	   proc timeout (process-status proc) result (buffer-string))
+	(keyboard-quit))
       result)))

 (defun tramp-search-regexp (regexp)

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 10:07     ` Michael Albinus
@ 2020-03-12 10:21       ` Robert Pluim
  2020-03-12 10:33         ` Michael Albinus
  2020-04-14  6:56       ` Daniel Colascione
  1 sibling, 1 reply; 20+ messages in thread
From: Robert Pluim @ 2020-03-12 10:21 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Daniel Colascione, Stefan Monnier, emacs-devel

>>>>> On Thu, 12 Mar 2020 11:07:29 +0100, Michael Albinus <michael.albinus@gmx.de> said:

    Michael> Daniel Colascione <dancol@dancol.org> writes:
    >>> Are there bug#NNN where these are discussed?
    >> 
    >> No. The motiviation is Tramp inflooping after a quit because it uses
    >> with-local-quit inside tramp-accept-process-output, which causes
    >> tramp-wait-for-regexp to retry infinitely and quickly.
    >> 
    >> I found *that* behavior trying to figure out why M-x compile over
    >> Tramp was hanging not only Emacs, but also the SSH connection more
    >> generally. I *think* it has something to do with compilation-filter
    >> calling file-truename, which invokes Tramp's handler, which wants to
    >> talk over the same SSH connection that's spewing compilation messages,
    >> deadlocking something somewhere.

    Michael> Please write a bug report, it should be solved in Tramp.

    Michael> with-local-quit is used in tramp-accept-process-output by
    Michael> intention. There were reports that Tramp hung, and couldn't be quit ...

    Michael> Would the appended patch help (completely untested)?

Yes! This solves my 'tramp hangs when visiting a patch' hang (or at
least, I can now C-g out of it).

Robert



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 10:21       ` Robert Pluim
@ 2020-03-12 10:33         ` Michael Albinus
  2020-03-13 10:00           ` Michael Albinus
  0 siblings, 1 reply; 20+ messages in thread
From: Michael Albinus @ 2020-03-12 10:33 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Daniel Colascione, Stefan Monnier, emacs-devel

Robert Pluim <rpluim@gmail.com> writes:

Hi Robert,

>     Michael> with-local-quit is used in tramp-accept-process-output by
>     Michael> intention. There were reports that Tramp hung, and
>     Michael> couldn't be quit ...
>
>     Michael> Would the appended patch help (completely untested)?
>
> Yes! This solves my 'tramp hangs when visiting a patch' hang (or at
> least, I can now C-g out of it).

Thanks for the feedback. I'm waiting for Daniel, and if he also confirms
an improvement, I'll commit.

> Robert

Best regards, Michael.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12  2:20 ` Stefan Monnier
  2020-03-12  2:32   ` Daniel Colascione
  2020-03-12  3:29   ` Óscar Fuentes
@ 2020-03-12 13:31   ` Richard Copley
  2020-03-12 17:36     ` Drew Adams
  2020-03-12 22:06     ` Stefan Monnier
  2 siblings, 2 replies; 20+ messages in thread
From: Richard Copley @ 2020-03-12 13:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Emacs Development

Stefan, will you please consider configuring your system to send email
to the lists as plain text? Recently your messages have "Content-type:
text/markdown". My user agent presents their body as an attachment,
which is inconvenient.

On Thu, 12 Mar 2020 at 02:21, Stefan Monnier <monnier@iro.umontreal.ca> wrote:



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 13:31   ` Richard Copley
@ 2020-03-12 17:36     ` Drew Adams
  2020-03-12 22:06     ` Stefan Monnier
  1 sibling, 0 replies; 20+ messages in thread
From: Drew Adams @ 2020-03-12 17:36 UTC (permalink / raw)
  To: Richard Copley, Stefan Monnier; +Cc: Emacs Development

> Stefan, will you please consider configuring your system to send email
> to the lists as plain text? Recently your messages have "Content-type:
> text/markdown". My user agent presents their body as an attachment,
> which is inconvenient.

+1.  I was going to say the same thing.

FYI, I think that the use of an attachment started
about a week ago.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  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-14 15:31       ` Drew Adams
  1 sibling, 2 replies; 20+ messages in thread
From: Stefan Monnier @ 2020-03-12 22:06 UTC (permalink / raw)
  To: Richard Copley; +Cc: Emacs Development

> Stefan, will you please consider configuring your system to send email
> to the lists as plain text? Recently your messages have "Content-type:
> text/markdown".

Hmm... that's odd... I did that a few months ago but then had to revert
it because of the number of broken MUAs out there.
Can you forward me one of the recent ones?

> My user agent presents their body as an attachment, which
> is inconvenient.

I'd urge you to report this as the bug that it is (MIME says very
clearly that any `text/<foo>` that's not specifically supported by the
MUA should be rendered as `text/plain`).


        Stefan




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 22:06     ` Stefan Monnier
@ 2020-03-12 23:14       ` Richard Copley
  2020-03-12 23:35         ` Stefan Monnier
  2020-03-14 15:31       ` Drew Adams
  1 sibling, 1 reply; 20+ messages in thread
From: Richard Copley @ 2020-03-12 23:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Emacs Development

Thanks, your latest messages are being displayed properly here.

On Thu, 12 Mar 2020 at 22:06, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > Stefan, will you please consider configuring your system to send email
> > to the lists as plain text? Recently your messages have "Content-type:
> > text/markdown".
>
> Hmm... that's odd... I did that a few months ago but then had to revert
> it because of the number of broken MUAs out there.
> Can you forward me one of the recent ones?

Sent off-list.

> > My user agent presents their body as an attachment, which
> > is inconvenient.
>
> I'd urge you to report this as the bug that it is (MIME says very
> clearly that any `text/<foo>` that's not specifically supported by the
> MUA should be rendered as `text/plain`).

It's the Google Mail web interface. Unfortunately, as far as I know,
they don't solicit bug reports.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 23:14       ` Richard Copley
@ 2020-03-12 23:35         ` Stefan Monnier
  2020-03-12 23:56           ` Richard Copley
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2020-03-12 23:35 UTC (permalink / raw)
  To: Richard Copley; +Cc: Emacs Development

> It's the Google Mail web interface.  Unfortunately, as far as I know,
> they don't solicit bug reports.

A quick search turned up

    https://www.wikihow.com/Report-a-Bug-in-Gmail


-- Stefan




^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 23:35         ` Stefan Monnier
@ 2020-03-12 23:56           ` Richard Copley
  0 siblings, 0 replies; 20+ messages in thread
From: Richard Copley @ 2020-03-12 23:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Emacs Development

On Thu, 12 Mar 2020 at 23:35, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > It's the Google Mail web interface.  Unfortunately, as far as I know,
> > they don't solicit bug reports.
>
> A quick search turned up
>
>     https://www.wikihow.com/Report-a-Bug-in-Gmail

Oh thanks, that's good. Feedback submitted.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 10:33         ` Michael Albinus
@ 2020-03-13 10:00           ` Michael Albinus
  0 siblings, 0 replies; 20+ messages in thread
From: Michael Albinus @ 2020-03-13 10:00 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Daniel Colascione, Stefan Monnier, emacs-devel

Michael Albinus <michael.albinus@gmx.de> writes:

Hi,

>>     Michael> with-local-quit is used in tramp-accept-process-output by
>>     Michael> intention. There were reports that Tramp hung, and
>>     Michael> couldn't be quit ...
>>
>>     Michael> Would the appended patch help (completely untested)?
>>
>> Yes! This solves my 'tramp hangs when visiting a patch' hang (or at
>> least, I can now C-g out of it).
>
> Thanks for the feedback. I'm waiting for Daniel, and if he also confirms
> an improvement, I'll commit.

No answer yet, so I have pushed it to the master branch. My internal
tests haven't shown any regression.

>> Robert

Best regards, Michael.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 22:06     ` Stefan Monnier
  2020-03-12 23:14       ` Richard Copley
@ 2020-03-14 15:31       ` Drew Adams
  2020-03-14 18:21         ` Stefan Monnier
  1 sibling, 1 reply; 20+ messages in thread
From: Drew Adams @ 2020-03-14 15:31 UTC (permalink / raw)
  To: Stefan Monnier, Richard Copley; +Cc: Emacs Development

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

Attached.

> > Stefan, will you please consider configuring your system to send email
> > to the lists as plain text? Recently your messages have "Content-type:
> > text/markdown".
> 
> Hmm... that's odd... I did that a few months ago but then had to revert
> it because of the number of broken MUAs out there.
> Can you forward me one of the recent ones?

[-- Attachment #2: Type: message/rfc822, Size: 1620 bytes --]

From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Steve Youngs <steve@sxemacs.org>
Cc: emacs-devel@gnu.org, Richard Stallman <rms@gnu.org>, Dima Kogan <dima@secretsauce.net>
Subject: Re: Can we expand the valid location of "Local Variables" ?
Date: Sat, 14 Mar 2020 00:07:30 -0700 (PDT)
Message-ID: <jwv5zf7pgb5.fsf-monnier+emacs@gnu.org>


[-- Attachment #2.1: Untitled.txt --]
[-- Type: text/x-markdown, Size: 710 bytes --]

From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Steve Youngs <steve@sxemacs.org>
Cc: emacs-devel@gnu.org, Richard Stallman <rms@gnu.org>, Dima Kogan <dima@secretsauce.net>
Subject: Re: Can we expand the valid location of "Local Variables" ?
Date: Sat, 14 Mar 2020 00:07:30 -0700 (PDT)
Message-ID: <jwv5zf7pgb5.fsf-monnier+emacs@gnu.org>

> How about some form of "include" mechanism:
>
>         Local Variables:
>         @include: FILENAME
>         End:

Sounds OK, yes.

> I would suggest implementing that with the same care, considerations,
> and restrictions as has `eval' for Locals

Indeed.  There be dragons.

> Mind you, if your Local Vars sections are getting so out of hand that
> they warrant having a whole extra file to store them, you should probably
> re-think what you're doing. :-)

I think such an @include directive can be useful not just for large
sections but also to avoid duplicating settings (i.e. a kind of
alternative to .dir-locals.el, not tied to the directory hierarchy).


        Stefan



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-14 15:31       ` Drew Adams
@ 2020-03-14 18:21         ` Stefan Monnier
  2020-03-14 18:36           ` Drew Adams
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Monnier @ 2020-03-14 18:21 UTC (permalink / raw)
  To: Drew Adams; +Cc: Richard Copley, Emacs Development

> Attached.

OK, I found the culprit.


        Stefan




^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-14 18:21         ` Stefan Monnier
@ 2020-03-14 18:36           ` Drew Adams
  0 siblings, 0 replies; 20+ messages in thread
From: Drew Adams @ 2020-03-14 18:36 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Richard Copley, Emacs Development

> > Attached.
> 
> OK, I found the culprit.

Great!  Thanks for doing that.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-03-12 10:07     ` Michael Albinus
  2020-03-12 10:21       ` Robert Pluim
@ 2020-04-14  6:56       ` Daniel Colascione
  2020-04-14  8:40         ` Michael Albinus
  1 sibling, 1 reply; 20+ messages in thread
From: Daniel Colascione @ 2020-04-14  6:56 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Stefan Monnier, emacs-devel

On 3/12/20 3:07 AM, Michael Albinus wrote:
> Daniel Colascione <dancol@dancol.org> writes:
> 
>>> Are there bug#NNN where these are discussed?
>>
>> No. The motiviation is Tramp inflooping after a quit because it uses
>> with-local-quit inside tramp-accept-process-output, which causes
>> tramp-wait-for-regexp to retry infinitely and quickly.
>>
>> I found *that* behavior trying to figure out why M-x compile over
>> Tramp was hanging not only Emacs, but also the SSH connection more
>> generally. I *think* it has something to do with compilation-filter
>> calling file-truename, which invokes Tramp's handler, which wants to
>> talk over the same SSH connection that's spewing compilation messages,
>> deadlocking something somewhere.
> 
> Please write a bug report, it should be solved in Tramp.

No, it shouldn't. This use of with-local-quit can *never* be correct 
because a stray C-g can *always* interfere with a filter. Don't do 
long-running operations in process filters. Period. The fix isn't to 
make quitting these operations possible in some defined manner. The fix 
is not to do them in the first place.



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] Really prevent quitting in sensitive contexts
  2020-04-14  6:56       ` Daniel Colascione
@ 2020-04-14  8:40         ` Michael Albinus
  0 siblings, 0 replies; 20+ messages in thread
From: Michael Albinus @ 2020-04-14  8:40 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: Stefan Monnier, emacs-devel

Daniel Colascione <dancol@dancol.org> writes:

Hi Daniel,

> No, it shouldn't. This use of with-local-quit can *never* be correct
> because a stray C-g can *always* interfere with a filter. Don't do
> long-running operations in process filters. Period. The fix isn't to
> make quitting these operations possible in some defined manner. The
> fix is not to do them in the first place.

How do you want to prevent use of such a basic operation like
`file-truename' in a process filter? And yes, in the remote case this
could be a "long-running operation".

Best regards, Michael.



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-04-14  8:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-12  2:05 [PATCH] Really prevent quitting in sensitive contexts Daniel Colascione
2020-03-12  2:20 ` 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

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).