Re: A whole lotta auto-saving going

[Stefan Monnier <monnier@iro.umontreal.ca>]

>> Here's a repro from emacs -Q:
>>
>> (require 'package)
>> (let* ((no-ssl (and (memq system-type '(windows-nt ms-dos))
>>                     (not (gnutls-available-p))))
>>        (url (concat (if no-ssl "http" "https") "://melpa.org/packages/")))
>>   (add-to-list 'package-archives (cons "melpa" url) t))
>> (package-initialize)
>> (package-install 'enh-ruby-mode)
>> (enh-ruby-mode)
>> (auto-save-mode)
>>
>> Then start typing, it should flicker "Auto-saving...done" in the echo area
>
> Do you have a repro that doesn't involve installing outside packages?
> :-)  That would help a lot.

Yes, really, I mean, c'mon!  First you don't send a patch to fix the
problem and then to add insult to injury you taunt us with a recipe that
relies on external code?!

OK, more seriously: thanks for the recipe, I can now reproduce the
problem locally.

Indeed reversing my patch avoids the problem, and the subset of it below
is enough to avoid the problem.

The manifestation of the problem is the following: in reach_char, where
we do

       {
	  Lisp_Object tem0;
	  EMACS_INT timeout = XFIXNAT (Vauto_save_timeout);

	  timeout = min (timeout, MOST_POSITIVE_FIXNUM / delay_level * 4);
	  timeout = delay_level * timeout / 4;
	  ptrdiff_t count1 = SPECPDL_INDEX ();
	  save_getcjmp (save_jump);
	  record_unwind_protect_ptr (restore_getcjmp, save_jump);
	  restore_getcjmp (local_getcjmp);
	  tem0 = sit_for (make_fixnum (timeout), 1, 1);
	  unbind_to (count1, Qnil);

	  if (EQ (tem0, Qt)
	      && ! CONSP (Vunread_command_events))
	    {
	      Fdo_auto_save (auto_save_no_message ? Qt : Qnil, Qnil);
	      redisplay ();
	    }
	}

the sit_for ends up returning "immediately" and with a value Qt.
I suspect this is because enh-ruby-mode receives process output during
the sit_for (which is what causes the "immediate" return) and the value
returned is Qt because there indeed no pending input.

sit_for waits with:

    wait_reading_process_output (sec, nsec, reading ? -1 : 1, do_display,
                                 Qnil, NULL, 0);

AFAIK with the patch below the sit_for also returns "immediately" but it
returns Qnil instead of Qt, and I assume this is because there is now
a BUFFER_SWITCH_EVENT in the queue.  If that's indeed what was
happening, then I think that the call to `record_asynch_buffer_change`
might have been a kind of "lucky workaround" and what should have happened
instead is that `wait_reading_process_output` should not return that
quickly (it's called with a 30s timeout argument).

So the way I see it the question is: why does
`wait_reading_process_output` return "immediately" when there's no
pending input and the timeout argument says 30s?


        Stefan



diff --git a/src/keyboard.c b/src/keyboard.c
index 9ee4c4f6d6..24402fa30c 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3430,7 +3433,8 @@ readable_events (int flags)
 		       && event->ie.part == scroll_bar_handle
 		       && event->ie.modifiers == 0)
 #endif
-		 )
+		  && !((flags & READABLE_EVENTS_FILTER_EVENTS)
+		       && event->kind == BUFFER_SWITCH_EVENT))
 		return 1;
 	      event = next_kbd_event (event);
 	    }
@@ -3581,6 +3585,12 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
 	  return;
 	}
     }
+  /* Don't insert two BUFFER_SWITCH_EVENT's in a row.
+     Just ignore the second one.  */
+  else if (event->kind == BUFFER_SWITCH_EVENT
+	   && kbd_fetch_ptr != kbd_store_ptr
+	   && prev_kbd_event (kbd_store_ptr)->kind == BUFFER_SWITCH_EVENT)
+    return;
 
   /* Don't let the very last slot in the buffer become full,
      since that would make the two pointers equal,
@@ -3614,6 +3624,7 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
     case ICONIFY_EVENT: ignore_event = Qiconify_frame; break;
     case DEICONIFY_EVENT: ignore_event = Qmake_frame_visible; break;
     case SELECTION_REQUEST_EVENT: ignore_event = Qselection_request; break;
+    case BUFFER_SWITCH_EVENT: ignore_event = Qbuffer_switch; break;
     default: ignore_event = Qnil; break;
     }
 
@@ -3953,6 +3964,7 @@ kbd_buffer_get_event (KBOARD **kbp,
 #ifdef HAVE_XWIDGETS
       case XWIDGET_EVENT:
 #endif
+      case BUFFER_SWITCH_EVENT:
       case SAVE_SESSION_EVENT:
       case NO_EVENT:
       case HELP_EVENT:
@@ -5332,6 +5344,14 @@ make_lispy_event (struct input_event *event)
       return list2 (Qmove_frame, list1 (event->frame_or_window));
 #endif
 
+    case BUFFER_SWITCH_EVENT:
+      {
+	/* The value doesn't matter here; only the type is tested.  */
+	Lisp_Object obj;
+        XSETBUFFER (obj, current_buffer);
+        return obj;
+      }
+
     /* Just discard these, by returning nil.
        With MULTI_KBOARD, these events are used as placeholders
        when we need to randomly delete events from the queue.
@@ -6788,6 +6808,41 @@ get_input_pending (int flags)
   return input_pending;
 }
 
+/* Put a BUFFER_SWITCH_EVENT in the buffer
+   so that read_key_sequence will notice the new current buffer.  */
+
+void
+record_asynch_buffer_change (void)
+{
+  /* We don't need a buffer-switch event unless Emacs is waiting for input.
+     The purpose of the event is to make read_key_sequence look up the
+     keymaps again.  If we aren't in read_key_sequence, we don't need one,
+     and the event could cause trouble by messing up (input-pending-p).
+     Note: Fwaiting_for_user_input_p always returns nil when async
+     subprocesses aren't supported.  */
+  if (!NILP (Fwaiting_for_user_input_p ()))
+    {
+      struct input_event event;
+
+      EVENT_INIT (event);
+      event.kind = BUFFER_SWITCH_EVENT;
+      event.frame_or_window = Qnil;
+      event.arg = Qnil;
+
+      /* Make sure no interrupt happens while storing the event.  */
+#ifdef USABLE_SIGIO
+      if (interrupt_input)
+	kbd_buffer_store_event (&event);
+      else
+#endif
+	{
+	  stop_polling ();
+	  kbd_buffer_store_event (&event);
+	  start_polling ();
+	}
+    }
+}
+
 /* Read any terminal input already buffered up by the system
    into the kbd_buffer, but do not wait.
 
@@ -11521,6 +11576,8 @@ syms_of_keyboard (void)
   /* Menu and tool bar item parts.  */
   DEFSYM (Qmenu_enable, "menu-enable");
 
+  DEFSYM (Qbuffer_switch, "buffer-switch");
+
 #ifdef HAVE_NTGUI
   DEFSYM (Qlanguage_change, "language-change");
   DEFSYM (Qend_session, "end-session");
diff --git a/src/keyboard.h b/src/keyboard.h
index 8bdffaa2bf..74b7d8517f 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -446,6 +446,7 @@ #define EVENT_HEAD_KIND(event_head) \
 extern void push_frame_kboard (struct frame *);
 extern void pop_kboard (void);
 extern void temporarily_switch_to_single_kboard (struct frame *);
+extern void record_asynch_buffer_change (void);
 extern void input_poll_signal (int);
 extern void start_polling (void);
 extern void stop_polling (void);
diff --git a/src/process.c b/src/process.c
index 67e930e18f..ee238ab307 100644
--- a/src/process.c
+++ b/src/process.c
@@ -6130,6 +6130,18 @@ read_and_dispose_of_process_output (struct Lisp_Process *p, char *chars,
   /* Restore waiting_for_user_input_p as it was
      when we were called, in case the filter clobbered it.  */
   waiting_for_user_input_p = waiting;
+
+#if 0 /* Call record_asynch_buffer_change unconditionally,
+	 because we might have changed minor modes or other things
+	 that affect key bindings.  */
+  if (! EQ (Fcurrent_buffer (), obuffer)
+      || ! EQ (current_buffer->keymap, okeymap))
+#endif
+    /* But do it only if the caller is actually going to read events.
+       Otherwise there's no need to make him wake up, and it could
+       cause trouble (for example it would make sit_for return).  */
+    if (waiting_for_user_input_p == -1)
+      record_asynch_buffer_change ();
 }
 
 DEFUN ("internal-default-process-filter", Finternal_default_process_filter,
diff --git a/src/termhooks.h b/src/termhooks.h
index 85a47c071b..dd59c66ae2 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -159,6 +159,7 @@ #define EMACS_TERMHOOKS_H
   SELECTION_REQUEST_EVENT,	/* Another X client wants a selection from us.
 				   See `struct selection_input_event'.  */
   SELECTION_CLEAR_EVENT,	/* Another X client cleared our selection.  */
+  BUFFER_SWITCH_EVENT,		/* A process filter has switched buffers.  */
   DELETE_WINDOW_EVENT,		/* An X client said "delete this window".  */
 #ifdef HAVE_NTGUI
   END_SESSION_EVENT,		/* The user is logging out or shutting down.  */