From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: A whole lotta auto-saving going Date: Sun, 10 Jan 2021 12:18:28 -0500 Message-ID: References: <8735zdyly0.fsf@gnus.org> <87y2h1vyhq.fsf@gnus.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="17414"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Aaron Jensen , emacs-devel@gnu.org To: Lars Ingebrigtsen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Jan 10 18:21:20 2021 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kyeP2-0004Pr-FD for ged-emacs-devel@m.gmane-mx.org; Sun, 10 Jan 2021 18:21:20 +0100 Original-Received: from localhost ([::1]:42800 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kyeP1-00007R-GX for ged-emacs-devel@m.gmane-mx.org; Sun, 10 Jan 2021 12:21:19 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:38386) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kyeMO-0006ML-1r for emacs-devel@gnu.org; Sun, 10 Jan 2021 12:18:36 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:5469) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kyeML-0003zM-86 for emacs-devel@gnu.org; Sun, 10 Jan 2021 12:18:35 -0500 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id A9BB7440BB8; Sun, 10 Jan 2021 12:18:31 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 72100440B7B; Sun, 10 Jan 2021 12:18:29 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1610299109; bh=KX4lsDqqm9NpQ2e63d2LfKXDBhrRa2ruZfk+VakYkK4=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=aUvOFkdBqJ/avPLBPaGO2vzNXcNGg1diL5x3QuAIbZonBhcfIHngkHB4lkGRqkvcB y1qon9drQclHjZ9FnQKHOa8YcXQzNnVKKKwp3p+O45PvboAsAmjoUz9sugW0/GNjp9 I/3d5ok8rsHovL1XVsqDB+aoHniVPIAY8t93SyY5jwCogK3juRoEES8uUc0x6HW9wo TccR9P5rKnZH8YB6iwjOJ+aqf161nyaecETARrFLyLXlZAxaYXie/Y7EjagGtKjpoK dWX1F1YbxZKxEcFoXJNcwr8/b2XHmYidtOtwrVpQ2d9/c6XZvu+2zxgf/UFvwoiDTC vLpABieBNjURA== Original-Received: from alfajor (unknown [45.72.224.181]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 2EA5A12023F; Sun, 10 Jan 2021 12:18:29 -0500 (EST) In-Reply-To: <87y2h1vyhq.fsf@gnus.org> (Lars Ingebrigtsen's message of "Sun, 10 Jan 2021 12:10:25 +0100") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:262863 Archived-At: >> 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. */