From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Noam Postavsky Newsgroups: gmane.emacs.bugs Subject: bug#36591: 26.2; Term's pager seems broken Date: Tue, 23 Jul 2019 22:07:24 -0400 Message-ID: <87imrsw5gj.fsf@gmail.com> References: <87lfwox3fi.fsf@gmail.com> <838ssops2u.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="15500"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2.90 (gnu/linux) Cc: abliss@gmail.com, 36591@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Jul 24 04:08:09 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hq6hN-0003vo-85 for geb-bug-gnu-emacs@m.gmane.org; Wed, 24 Jul 2019 04:08:09 +0200 Original-Received: from localhost ([::1]:48194 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hq6hL-0004zs-JY for geb-bug-gnu-emacs@m.gmane.org; Tue, 23 Jul 2019 22:08:07 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49524) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hq6hH-0004zY-KZ for bug-gnu-emacs@gnu.org; Tue, 23 Jul 2019 22:08:04 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hq6hG-0007Xk-6a for bug-gnu-emacs@gnu.org; Tue, 23 Jul 2019 22:08:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:55280) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hq6hG-0007Xe-0n for bug-gnu-emacs@gnu.org; Tue, 23 Jul 2019 22:08:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hq6hF-0003fr-QO for bug-gnu-emacs@gnu.org; Tue, 23 Jul 2019 22:08:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Noam Postavsky Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 24 Jul 2019 02:08:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36591 X-GNU-PR-Package: emacs Original-Received: via spool by 36591-submit@debbugs.gnu.org id=B36591.156393405414090 (code B ref 36591); Wed, 24 Jul 2019 02:08:01 +0000 Original-Received: (at 36591) by debbugs.gnu.org; 24 Jul 2019 02:07:34 +0000 Original-Received: from localhost ([127.0.0.1]:35868 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hq6go-0003fC-Bf for submit@debbugs.gnu.org; Tue, 23 Jul 2019 22:07:34 -0400 Original-Received: from mail-io1-f51.google.com ([209.85.166.51]:43709) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hq6gm-0003ex-Da for 36591@debbugs.gnu.org; Tue, 23 Jul 2019 22:07:32 -0400 Original-Received: by mail-io1-f51.google.com with SMTP id k20so86157802ios.10 for <36591@debbugs.gnu.org>; Tue, 23 Jul 2019 19:07:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=idHm9N47XEyOeOfjnXBvKAlyJ4hl4RgCBn3xebX+diI=; b=oS3bbYojuS/YCCwDxydSDDOYVPmEx32UJJHGR3x0a4Pbs6DGZSaPDGBy5WrHlIqi2K 7a+1dMdMCldenh98+v10yl6yks5UNjhjRAi1SvZCf3L3u1MnSE4PVKkTAiZ+F8qqPu4J ModbQpNFXtLY2saIVWFkmT1qDAx/Sjy8YWzhpOPZSRiEoqYiC0HbwDew4536wbtIslQx 8VpBD3KD1KEw7e923VC0oskra3LFB36xY9EpXVipcp3DCYZPqXwDCsv7PWIy6gyPuAIX c5FPx+yJtG8amcoHnN0R6eZBBHTDiE0XXsC0S5NAjLE9icDn/3tASWUxflhIrpi9iv36 h3VA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=idHm9N47XEyOeOfjnXBvKAlyJ4hl4RgCBn3xebX+diI=; b=OJPrqJD3eouoPvIT6B9F70WpLwxV1+gh7CVzlpRNWiBJuEhf4nR+i8QTRQiy0p0R3Q OcyFNt+WaUhSyQdIgK5lt9/Lya6/XtmdtdACFCRK6s8gvXJ7BkBouxlOWXmKMOWj524j 48257aQJMepiTIRiiCjPWauVe4fH0VCOnPPGuiIpukE39lqC2rHkbyHpeY+pIDJwwB1v eBtqCO01uE6/ADvgaDK6Pr4mXtTeJdkZl5kE2Da/TDiglQkk/z5WDXVYAhXTVUslRhRP CL7JbC+rvQZDn8QI37jZW2HC9la7GUm0mXLZyqom3IDXX8ebvC7g7r91aeA/tKrGCnnq Pydg== X-Gm-Message-State: APjAAAWXs8PbucxwxiGTvM+zuN1nCDqRd4BFNBhCl1dsIt/5pNid7NW4 Gas0mztvTyTwPxeE04l1B+SfE5j6 X-Google-Smtp-Source: APXvYqxDIjVmnxoxB8RJHsMiJGGQLKBF4FP746UlFGl4wSXTUsswTzt4X8K14vlNLBhLSBwZDLUKwg== X-Received: by 2002:a02:13c3:: with SMTP id 186mr80964913jaz.30.1563934046605; Tue, 23 Jul 2019 19:07:26 -0700 (PDT) Original-Received: from minid (cbl-45-2-119-34.yyz.frontiernetworks.ca. [45.2.119.34]) by smtp.gmail.com with ESMTPSA id t133sm65945121iof.21.2019.07.23.19.07.24 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 23 Jul 2019 19:07:25 -0700 (PDT) In-Reply-To: <838ssops2u.fsf@gnu.org> (Eli Zaretskii's message of "Tue, 23 Jul 2019 20:40:41 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:163658 Archived-At: --=-=-= Content-Type: text/plain Eli Zaretskii writes: > I don't think I understand why the change fixes the bug, sorry. Can > you tell in more detail? Okay. First, see attached reduced test case which demonstrates the bug. --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=bug-36591-proc-filter-t.el Content-Description: demo for bug (with-current-buffer (get-buffer-create "*test buffer*") (let* ((oldproc (get-buffer-process (current-buffer)))) (when oldproc (kill-process oldproc) (accept-process-output oldproc))) (erase-buffer) (display-buffer (current-buffer)) (let* ((print-level nil) (print-length nil) (proc (start-process "test proc" (current-buffer) (concat invocation-directory invocation-name) "-Q" "--batch" "--eval" (prin1-to-string '(let (s) (while (setq s (read-from-minibuffer "$ ")) (princ s) (princ "\n"))))))) (send-string proc "one\n") (sit-for 0.5) ; Read "one". (set-process-filter proc t) ; Stop reading from proc. (send-string proc "two\n") (sit-for 1) ; Can't read "two" yet. (set-process-filter proc ; Resume reading from proc. #'internal-default-process-filter) (sit-for 0.5) ; Read "two" from proc. )) --=-=-= Content-Type: text/plain In Emacs 25 it shows a buffer named "*test buffer*" with contents $ one $ two $ In Emacs 26, the "two" never gets read, so the result is $ one $ Calling (set-process-filter PROC t) is supposed to turn off reading for PROC. This works fine. But (set-process-filter PROC FILTER) doesn't turn on reading again in Emacs 26. Neither of the cases in set_process_filter_masks are taken in the case when FILTER is not t. DEFUN ("set-process-filter", Fset_process_filter, Sset_process_filter, ... (Lisp_Object process, Lisp_Object filter) { ... pset_filter (p, filter); // <-- sets p->filter = filter; if (p->infd >= 0) set_process_filter_masks (p); ... } static void set_process_filter_masks (struct Lisp_Process *p) { if (EQ (p->filter, Qt) && !EQ (p->status, Qlisten)) delete_read_fd (p->infd); else if (EQ (p->filter, Qt) /* Network or serial process not stopped: */ && !EQ (p->command, Qt)) add_process_read_fd (p->infd); } In Emacs 25 it looks like the 'if' cases are the same, but there is a subtle difference: the first 'if' checks 'filter', while the second checks 'p->filter'. And furthermore note that pset_filter is called after this check against 'p->filter', so it is checking the "original" 'p->filter' value (which means that Emacs 25 has a bug that the fd is incorrectly enabled if setting the filter to t twice, i.e., (progn (set-process-filter PROC t) (set-process-filter PROC t))). DEFUN ("set-process-filter", Fset_process_filter, Sset_process_filter, ... (register Lisp_Object process, Lisp_Object filter) { ... if (p->infd >= 0) { if (EQ (filter, Qt) && !EQ (p->status, Qlisten)) { FD_CLR (p->infd, &input_wait_mask); FD_CLR (p->infd, &non_keyboard_wait_mask); } else if (EQ (p->filter, Qt) /* Network or serial process not stopped: */ && !EQ (p->command, Qt)) { FD_SET (p->infd, &input_wait_mask); FD_SET (p->infd, &non_keyboard_wait_mask); } } pset_filter (p, filter); The patch found by Adam's bisect put the pset_filter call before this check, so that Emacs 26 checks the 'p->filter' after it's been set (i.e., the value of the 'filter' parameter). So the second case is no longer entered when calling (set-filter-process PROC FILTER). > Also, the same function is called in another > place, so what will this change do to that other caller? Hmm, it's difficult to say, there are a lot of optional code paths. I suspect socket subprocesses might suffer from the same bug, but there are also some (redundant?) calls add_process_read_fd that might cover it up (notice that all calls are guarded by !EQ (p->filter, Qt), i.e., the corrected version of the check in set_process_filter_masks). static void connect_network_socket (Lisp_Object proc, Lisp_Object addrinfos, Lisp_Object use_external_socket_p) { ... if (p->is_non_blocking_client) {...} else /* A server may have a client filter setting of Qt, but it must still listen for incoming connects unless it is stopped. */ if ((!EQ (p->filter, Qt) && !EQ (p->command, Qt)) || (EQ (p->status, Qlisten) && NILP (p->command))) add_process_read_fd (inch); ... } ... static void server_accept_connection (Lisp_Object server, int channel) { ... /* Client processes for accepted connections are not stopped initially. */ if (!EQ (p->filter, Qt)) add_process_read_fd (s); ... } ... int wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd, bool do_display, Lisp_Object wait_for_cell, struct Lisp_Process *wait_proc, int just_wait_proc) { ... if (0 <= p->infd && !EQ (p->filter, Qt) && !EQ (p->command, Qt)) add_process_read_fd (p->infd); ... } ... DEFUN ("continue-process", Fcontinue_process, Scontinue_process, 0, 2, 0, ...) (Lisp_Object process, Lisp_Object current_group) { ... if (EQ (p->command, Qt) && p->infd >= 0 && (!EQ (p->filter, Qt) || EQ (p->status, Qlisten))) { add_process_read_fd (p->infd); > And how come we lived with this bug for 3 years without having noticed > it? I think there is very little use of t as a process filter value. Maybe term.el is the only user of it. As to why nobody noticed the problem in term.el earlier, I have the impression that most users just assume term.el will be buggy and don't even bother reporting problems with it (typical suggestions are "run screen or tmux to handle escape codes properly", or "use the libvterm Emacs extension instead"). --=-=-=--