From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Ian Jackson Newsgroups: gmane.emacs.bugs Subject: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages] Date: Sat, 22 Jan 2022 19:34:33 +0000 Message-ID: <25068.23625.512978.147194@chiark.greenend.org.uk> References: <25067.17249.604070.872185@chiark.greenend.org.uk> <838rv8nua8.fsf@gnu.org> <87r190wqo1.fsf@yahoo.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="16056"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 53432@debbugs.gnu.org To: Eli Zaretskii , Po Lu Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Jan 22 20:36:11 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1nBMBG-0003wD-OO for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 22 Jan 2022 20:36:10 +0100 Original-Received: from localhost ([::1]:58838 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nBMBF-0003Dt-FR for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 22 Jan 2022 14:36:09 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:46454) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nBMAB-0003Dc-6o for bug-gnu-emacs@gnu.org; Sat, 22 Jan 2022 14:35:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:45064) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nBMAA-0003Hl-N8 for bug-gnu-emacs@gnu.org; Sat, 22 Jan 2022 14:35:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nBMAA-0004T7-Ko for bug-gnu-emacs@gnu.org; Sat, 22 Jan 2022 14:35:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Ian Jackson Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 22 Jan 2022 19:35:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 53432 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 53432-submit@debbugs.gnu.org id=B53432.164288008017129 (code B ref 53432); Sat, 22 Jan 2022 19:35:02 +0000 Original-Received: (at 53432) by debbugs.gnu.org; 22 Jan 2022 19:34:40 +0000 Original-Received: from localhost ([127.0.0.1]:37963 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nBM9n-0004SD-D7 for submit@debbugs.gnu.org; Sat, 22 Jan 2022 14:34:39 -0500 Original-Received: from permutation-city.chiark.greenend.org.uk ([212.13.197.230]:53966 helo=chiark.greenend.org.uk ident=Debian-exim) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nBM9l-0004S2-Eg for 53432@debbugs.gnu.org; Sat, 22 Jan 2022 14:34:38 -0500 Original-Received: by chiark.greenend.org.uk (Debian Exim 4.89 #1) with local (return-path ijackson@chiark.greenend.org.uk) id 1nBM9h-0006Tf-Ls; Sat, 22 Jan 2022 19:34:33 +0000 In-Reply-To: <87r190wqo1.fsf@yahoo.com>, <838rv8nua8.fsf@gnu.org> X-Mailer: VM 8.2.0b under 24.4.1 (i586-pc-linux-gnu) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:224860 Archived-At: Eli Zaretskii writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy"): > Thanks. I doubt that we want to complicate our input handling this > way on behalf of "abusing" inotify. File notifications are known not > to be scalable enough to support global-auto-revert-mode well. The problem is that keystrokes (and file notifications) get lost. I don't think we should be shipping code that can do that. Even if it only does it if you are working on a program whose build system writes a lot of files, and/or have a slow computer, and/or are unlucky. Note that magit turns on global-auto-revert-mode so this is a very common configuration. Loss of file updates is also a real problem, becuase it can lead to the user editing an old version of a file. The fact that I have to "abuse" things to demonstrate the race, does not mean that it's not a real bug. That's in the nature of race bugs. It's probably happening to the occasional user right now, albeit very rarely, and they probably think they just didn't hit the key properly or something. I really dislike the idea that emacs is making our users doubt themselves, by pretending the user didn't hit a key. I don't think it's good enough for this to be "rare" or "only in unusual situations" or "only when the computer is overloaded". We expect emacs to be reliable and we need to make it so. I'm obviously open to other suggestions for how to deal with this to make sure that both keystrokes and file updates are not *lost*, only possibly *delayed*. I don't really know the code so it is entirely plausible that my approach is wrong. Po Lu writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy"): > I would rather not allocate anything on the heap to hold inotify events. I'm not sure what your underlying objection is to this, so I will guess: I think you are concerned at the performance implications of allocating each time we handle inotify events. But my proposed code does not allocate each time. It only (re)allocates when it needs a bigger buffer than it already has. Of course the unbounded memory consumption is a problem, as I write in my FIXME. Allocating an undounded amount of memory on the stack each time, as the current code does, is really very brave and seems to me clearly worse than doing it amortised-approximately-once on the heap. (But maybe SAFE_ALLOCA falls back to malloc for big requests anyway.) > The best thing to do, IMO, would be to simply not store any > FILE_NOTIFY_EVENTS if the keyboard buffer is full, which shouldn't > affect normal use of file notifications, and would result in less change > to the keyboard code. Wouldn't that lead to file notifications getting lost ? I think that might result in buffers not being reverted, even though the user has global-auto-revert-mode enabled. As I say that would be a problem, because it can end up with the user editing the wrong version of a file. I don't think it is OK. If it is OK for file notifications to get lost, because the need for a buffer revert will be picked up another way somehow, then your suggestion makes perfect sense to me. But in that case, I don't understand why this code doesn't use a buffer of fixed size (or, at least, limited size). Or to put it another way, the current code does a hair-raising thing for which the only explanation I could think of was that it is aiming never to lose notifications; and if there's a comment somewhere saying that the whole inotify system is best-effort, and it's OK for it to drop things when stressed, I have missed it. If it is OK for it to be best-effort, then I think this needs to be in the documentation for inotify-add-watch. Currently it says The watch will look for ASPECT events and invoke CALLBACK when an event occurs. and there's nothing saying "it might not happen, so you must arrangee that this is not the sole way you are looking for changes". I think that'd be an API which would invite programmers to write and ship lost event bugs, especially since usually it would work just fine. But maybe it would e OK if it is almost always used only with expert reivew (eg, by code in other parts of emacs). Eli: > In any case, installing this on the emacs-28 branch is out of the > question: it's too late for that. We may install some variant of this > on master, after discussing how best to handle this. Po Lu's > suggestion to stop processing inotify events sounds better to me. I was advised by a friend that I ought to send patches against the emacs-28 branch, so I did that. Indeed, CONTRIBUTING says If you are fixing a bug that exists in the current release, you should generally commit it to the release branch; Anyway my current patches rebase cleanly from 27 all the way to master. It seems very likely that'll be true for whatever fix we come up with. So the fix can be forward- or back-ported as people like. > [Ian:] > > I hope the commit messages are in the expected format. I think I > > probably have GNU copyright paperwork on file already, perhaps under a > > different email address. > > I don't see your name or email address on file, so maybe they are both > different? My name hasn't changed. But I'm happy to do the paperwork (even if it would be "again"). If it did happen, it will have been a long long time ago. Ian. -- Ian Jackson These opinions are my own. Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk, that is a private address which bypasses my fierce spamfilter.