From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?B?Sm/Do28gVMOhdm9yYQ==?= Newsgroups: gmane.emacs.devel Subject: Re: emacs-29 8bf4cdcf79: Avoid recursive process filters in lisp/jsonrpc.el (bug#60088) Date: Sat, 17 Dec 2022 21:39:14 +0000 Message-ID: <877cypbth9.fsf@gmail.com> References: <167118072395.30479.8819833637573037468@vcs2.savannah.gnu.org> <20221216085204.43B07C04961@vcs2.savannah.gnu.org> <87h6xufv4v.fsf@neverwas.me> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="4420"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: F. Jason Park , emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Dec 17 22:38:38 2022 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 1p6etC-00011G-Ah for ged-emacs-devel@m.gmane-mx.org; Sat, 17 Dec 2022 22:38:38 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p6esW-0005eF-3d; Sat, 17 Dec 2022 16:37:56 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p6esU-0005e2-VA for emacs-devel@gnu.org; Sat, 17 Dec 2022 16:37:54 -0500 Original-Received: from mail-wm1-x335.google.com ([2a00:1450:4864:20::335]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p6esT-0001yf-5P for emacs-devel@gnu.org; Sat, 17 Dec 2022 16:37:54 -0500 Original-Received: by mail-wm1-x335.google.com with SMTP id ay40so4088207wmb.2 for ; Sat, 17 Dec 2022 13:37:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=1MbnwEPHWg1Uo0zCGHLyLdNN67/8NzFpv4mcXSxpUL4=; b=GdylS5JsDMeGJrzqJFyPF2r8JnRbjuuOOy75jeRpVv50UmbEagWkaowEQtrCatTY75 bpzenbzRMsBLq6VrnIMjX8alqghr/AtbM6eykJ8rijHFe7HONtC2z9im4XlwIGpgbbIK 8fz8wd83YpZyTtYgl8Q8/BJVkJSIId+CAYuYZdr5TlPgvCZkwaIQjffL/DknpfqAOxPU Uwo5mLzYTyFDtb7JMlXR4eIVl8A9EdrhtNl01zb0gjEVmrj7g8YfQfbmVVGX70qGppUg C7IyOKMpLh9fFucuHhHWfWrxI8tsGEuXJ0oMTd+qkNLynm6s0QjeNZ0j6rPBtKiY/3Lh IBQQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1MbnwEPHWg1Uo0zCGHLyLdNN67/8NzFpv4mcXSxpUL4=; b=ClTsNi18ZzcJlNOvRghhphuBnaKCFhxNdeJx0piqFuC4jh15e8fC2gQPa45nr9p3jy MASCkJ4Q357W29vg6/0YwglKnEEW6oZaVK8/GtpZiwsVTz6K4M4OaV5I046G4/w/FtHr n95G6ylyzJrQr1ewLEFIpQNJiEyTpo0mlnWKMf6dI/bpbQMI4qT1VP9Ajud+NneDNW7o XnqlVVfWK3dzGUSjSx/0KEsNpa54BPRFBzwbykPmD7P8oZEut7yIp73PzCTFIcpJJa31 eYKZF4a6jdV9IWnxKsNMmR/dzh2AxRzImSQSgVYWW+tPxWX3c8bHK6FPPmtn1urCW9UP nTtw== X-Gm-Message-State: ANoB5pnJUIIatGVjP9YZJdz9YWjWAe81iDv4JSkvrO1lktBS35Uxel+s 5c9e+AGxy9mMZoQ3rapzUOsuE/5ZrGc= X-Google-Smtp-Source: AA0mqf4gG+eVYaZFCIE54++8H5eXjIOAmZvORSSQgtl40Ao9Zs2ndU8UlLh+QwZtxEppFdnQHTCLIA== X-Received: by 2002:a05:600c:a0f:b0:3d0:8a0e:3910 with SMTP id z15-20020a05600c0a0f00b003d08a0e3910mr28776991wmp.34.1671313070390; Sat, 17 Dec 2022 13:37:50 -0800 (PST) Original-Received: from krug (87-196-74-4.net.novis.pt. [87.196.74.4]) by smtp.gmail.com with ESMTPSA id l7-20020a5d5607000000b002420fe50322sm5798748wrv.91.2022.12.17.13.37.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Dec 2022 13:37:49 -0800 (PST) In-Reply-To: (Stefan Monnier's message of "Sat, 17 Dec 2022 10:37:33 -0500") Received-SPF: pass client-ip=2a00:1450:4864:20::335; envelope-from=joaotavora@gmail.com; helo=mail-wm1-x335.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, 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.29 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-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:301591 Archived-At: Stefan Monnier writes: > F. Jason Park [2022-12-16 21:37:36] wrote: >> Stefan Monnier writes: >>>> Avoid recursive process filters in lisp/jsonrpc.el (bug#60088) >>> >>> BTW, this has bitten us in various other cases, we should fix it once >>> and for all in the C code by marking the process as "busy" while we're >>> running the filters so we never run filters recursively. >>> [ If we ever bump into a case where recursive filters are needed, we >>> can then add some function to remove the "busy" mark. >>> Calling (accept-process-output PROC) should naturally mark PROC as >>> not-busy. ] >>> >>> >>> Stefan >> >> A possibly related discussion from the bug archive: >> >> https://lists.gnu.org/archive/html/bug-gnu-emacs/2022-03/msg01211.html > > Indeed. I think this points to the need to "spawn" a piece of code to > be executed "ASAP" but not necessarily immediately. Would that be sth like (if in-process-filter (run-at-time 0 nil #'piece-of-code) (piece-of-code)) ? ... supposing in-process-filter existed, of course. > > This way when a process filter needs to send something in response to > what it received, it can just "spawn" the send, so we can return from > the process filter before the send finishes. I guess you can see it that way too. So there are two ways to solve this: * only process-send-input in process filters makes sense * all but process-send-input in process filters makes sense I'm more into of the first persuasion, but I think it shouldn't allow output to be accepted when called from within a process filter. But I guess the second option isn't bad too. I've rehearsed a patch to jsonrpc.el that uses this idea instead of the other kill recursion one, and maybe it's cleaner, indeed. Would appreciate feedback. diff --git a/lisp/jsonrpc.el b/lisp/jsonrpc.el index 2d562610b3..e72644d317 100644 --- a/lisp/jsonrpc.el +++ b/lisp/jsonrpc.el @@ -418,6 +418,9 @@ initialize-instance (setq buffer-read-only t)) (process-put proc 'jsonrpc-connection conn))) +(defvar jsonrpc--in-process-filter nil + "Non-nil if inside `jsonrpc--process-filter'.") + (cl-defmethod jsonrpc-connection-send ((connection jsonrpc-process-connection) &rest args &key @@ -437,13 +440,16 @@ jsonrpc-connection-send (headers `(("Content-Length" . ,(format "%d" (string-bytes json))) ;; ("Content-Type" . "application/vscode-jsonrpc; charset=utf-8") - ))) + )) + (send-it + (lambda () (process-send-string (jsonrpc--process connection) (cl-loop for (header . value) in headers concat (concat header ": " value "\r\n") into header-section finally return (format "%s\r\n%s" header-section json))) - (jsonrpc--log-event connection message 'client))) + (jsonrpc--log-event connection message 'client)))) + (if jsonrpc--in-process-filter (run-at-time 0 nil send-it) (funcall send-it)))) (defun jsonrpc-process-type (conn) "Return the `process-type' of JSONRPC connection CONN." @@ -548,22 +554,8 @@ jsonrpc--process-sentinel (delete-process proc) (funcall (jsonrpc--on-shutdown connection) connection))))) -(defvar jsonrpc--in-process-filter nil - "Non-nil if inside `jsonrpc--process-filter'.") - (cl-defun jsonrpc--process-filter (proc string) "Called when new data STRING has arrived for PROC." - (when jsonrpc--in-process-filter - ;; Problematic recursive process filters may happen if - ;; `jsonrpc--connection-receive', called by us, eventually calls - ;; client code which calls `process-send-string' (which see) to, - ;; say send a follow-up message. If that happens to writes enough - ;; bytes for pending output to be received, we will lose JSONRPC - ;; messages. In that case, remove recursiveness by re-scheduling - ;; ourselves to run from within a timer as soon as possible - ;; (bug#60088) - (run-at-time 0 nil #'jsonrpc--process-filter proc string) - (cl-return-from jsonrpc--process-filter)) (when (buffer-live-p (process-buffer proc)) (with-current-buffer (process-buffer proc) (let* ((inhibit-read-only t)