Carbon (macOS) port asks about killing stderr buffer

[David Edmondson]

After the changes to use `make-process', the Carbon port of emacs on
macOS (often referred to as emacs-mac or the railwaycat port) will ask
about killing the stderr buffer after any `notmuch-search':

Debugger entered--Lisp error: (quit)
yes-or-no-p("Buffer \" *notmuch-stderr*-839121\" has a running process; kill it? ")
process-kill-buffer-query-function()
kill-buffer(#<buffer *notmuch-stderr*-839121>)
notmuch-start-notmuch-sentinel(#<process notmuch-search> "finished\n")

A quick look at the implementation of `make-process' in the Carbon port
didn't reveal anything obvious to me. This mostly seems like a race -
whether emacs has decided that the process associated with the stderr
buffer is dead or not when we call `kill-buffer'. Is any ordering
guaranteed by the implementation?

I _think_ that have also seen the same problem when asynchronous address
harvesting is happening for completion on the default NextStep port for
macOS, but haven't been able to reliably reproduce it.

dme.

[David Edmondson]

Post by David Edmondson
After the changes to use `make-process', the Carbon port of emacs on
macOS (often referred to as emacs-mac or the railwaycat port) will ask
Debugger entered--Lisp error: (quit)
yes-or-no-p("Buffer \" *notmuch-stderr*-839121\" has a running process; kill it? ")
process-kill-buffer-query-function()
kill-buffer(#<buffer *notmuch-stderr*-839121>)
notmuch-start-notmuch-sentinel(#<process notmuch-search> "finished\n")

A hack-and-slash patch for this that works for me:

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 010be454..7c0faee4 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -909,7 +909,7 @@ invoke `set-process-sentinel' directly on the returned process,
as that will interfere with the handling of stderr and the exit
status."

- (let (err-file err-buffer proc
+ (let (err-file err-buffer proc err-proc
;; Find notmuch using Emacs' `exec-path'
(command (or (executable-find notmuch-command)
(error "Command not found: %s" notmuch-command))))
@@ -926,11 +926,13 @@ status."
:buffer buffer
:command (cons command args)
:connection-type 'pipe
- :stderr err-buffer))
+ :stderr err-buffer)
+ err-proc (get-buffer-process err-buffer))
(process-put proc 'err-buffer err-buffer)
- ;; Silence "Process NAME stderr finished" in stderr by adding a
- ;; no-op sentinel to the fake stderr process object
- (set-process-sentinel (get-buffer-process err-buffer) #'ignore))
+
+ (process-put err-proc 'err-file err-file)
+ (process-put err-proc 'err-buffer err-buffer)
+ (set-process-sentinel err-proc #'notmuch-start-notmuch-error-sentinel))

;; On Emacs versions before 25, there is no way to capture
;; stdout and stderr separately for asynchronous processes, or
@@ -990,9 +992,14 @@ status."
;; Emacs behaves strangely if an error escapes from a sentinel,
;; so turn errors into messages.
(message "%s" (error-message-string err))))
- (when err-buffer (kill-buffer err-buffer))
(when err-file (ignore-errors (delete-file err-file)))))

+(defun notmuch-start-notmuch-error-sentinel (proc event)
+ (let* ((err-file (process-get proc 'err-file))
+ (err-buffer (or (process-get proc 'err-buffer)
+ (find-file-noselect err-file))))
+ (when err-buffer (kill-buffer err-buffer))))
+
;; This variable is used only buffer local, but it needs to be
;; declared globally first to avoid compiler warnings.
(defvar notmuch-show-process-crypto nil)

dme.

[David Edmondson]

Post by David Edmondson
After the changes to use `make-process', the Carbon port of emacs on
macOS (often referred to as emacs-mac or the railwaycat port) will ask
Debugger entered--Lisp error: (quit)
yes-or-no-p("Buffer \" *notmuch-stderr*-839121\" has a running process; kill it? ")
process-kill-buffer-query-function()
kill-buffer(#<buffer *notmuch-stderr*-839121>)
notmuch-start-notmuch-sentinel(#<process notmuch-search> "finished\n")

This happens on the current version of OpenIndiana (a fork of a fork of
a fork of a fork of Solaris) as well when running:

(emacs-version)
"GNU Emacs 25.3.1 (i386-pc-solaris2.11, GTK+ Version 2.24.31)
of 2017-09-12"

Post by David Edmondson
A quick look at the implementation of `make-process' in the Carbon port
didn't reveal anything obvious to me. This mostly seems like a race -
whether emacs has decided that the process associated with the stderr
buffer is dead or not when we call `kill-buffer'. Is any ordering
guaranteed by the implementation?
I _think_ that have also seen the same problem when asynchronous address
harvesting is happening for completion on the default NextStep port for
macOS, but haven't been able to reliably reproduce it.
dme.
--
I got a girlfriend that's better than that.

dme.

[David Edmondson]

On some platforms (e.g. macOS), it is necessary to add a real sentinel
process for the error buffer used by `notmuch-start-notmuch' rather
than a no-op sentinel.
---
emacs/notmuch-lib.el | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index a7e02710..03c966b7 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -909,7 +909,7 @@ invoke `set-process-sentinel' directly on the returned process,
as that will interfere with the handling of stderr and the exit
status."

- (let (err-file err-buffer proc
+ (let (err-file err-buffer proc err-proc
;; Find notmuch using Emacs' `exec-path'
(command (or (executable-find notmuch-command)
(error "Command not found: %s" notmuch-command))))
@@ -926,11 +926,13 @@ status."
:buffer buffer
:command (cons command args)
:connection-type 'pipe
- :stderr err-buffer))
+ :stderr err-buffer)
+ err-proc (get-buffer-process err-buffer))
(process-put proc 'err-buffer err-buffer)
- ;; Silence "Process NAME stderr finished" in stderr by adding a
- ;; no-op sentinel to the fake stderr process object
- (set-process-sentinel (get-buffer-process err-buffer) #'ignore))
+
+ (process-put err-proc 'err-file err-file)
+ (process-put err-proc 'err-buffer err-buffer)
+ (set-process-sentinel err-proc #'notmuch-start-notmuch-error-sentinel))

;; On Emacs versions before 25, there is no way to capture
;; stdout and stderr separately for asynchronous processes, or
@@ -990,9 +992,14 @@ status."
;; Emacs behaves strangely if an error escapes from a sentinel,
;; so turn errors into messages.
(message "%s" (error-message-string err))))
- (when err-buffer (kill-buffer err-buffer))
(when err-file (ignore-errors (delete-file err-file)))))

+(defun notmuch-start-notmuch-error-sentinel (proc event)
+ (let* ((err-file (process-get proc 'err-file))
+ (err-buffer (or (process-get proc 'err-buffer)
+ (find-file-noselect err-file))))
+ (when err-buffer (kill-buffer err-buffer))))
+
;; This variable is used only buffer local, but it needs to be
;; declared globally first to avoid compiler warnings.
(defvar notmuch-show-process-crypto nil)

[David Edmondson]

This failure mode is very annoying. Could I persuade someone to review
the proposed change?

Post by David Edmondson
On some platforms (e.g. macOS), it is necessary to add a real sentinel
process for the error buffer used by `notmuch-start-notmuch' rather
than a no-op sentinel.
---
emacs/notmuch-lib.el | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index a7e02710..03c966b7 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -909,7 +909,7 @@ invoke `set-process-sentinel' directly on the returned process,
as that will interfere with the handling of stderr and the exit
status."
- (let (err-file err-buffer proc
+ (let (err-file err-buffer proc err-proc
;; Find notmuch using Emacs' `exec-path'
(command (or (executable-find notmuch-command)
(error "Command not found: %s" notmuch-command))))
@@ -926,11 +926,13 @@ status."
:buffer buffer
:command (cons command args)
:connection-type 'pipe
- :stderr err-buffer))
+ :stderr err-buffer)
+ err-proc (get-buffer-process err-buffer))
(process-put proc 'err-buffer err-buffer)
- ;; Silence "Process NAME stderr finished" in stderr by adding a
- ;; no-op sentinel to the fake stderr process object
- (set-process-sentinel (get-buffer-process err-buffer) #'ignore))
+
+ (process-put err-proc 'err-file err-file)
+ (process-put err-proc 'err-buffer err-buffer)
+ (set-process-sentinel err-proc #'notmuch-start-notmuch-error-sentinel))
;; On Emacs versions before 25, there is no way to capture
;; stdout and stderr separately for asynchronous processes, or
@@ -990,9 +992,14 @@ status."
;; Emacs behaves strangely if an error escapes from a sentinel,
;; so turn errors into messages.
(message "%s" (error-message-string err))))
- (when err-buffer (kill-buffer err-buffer))
(when err-file (ignore-errors (delete-file err-file)))))
+(defun notmuch-start-notmuch-error-sentinel (proc event)
+ (let* ((err-file (process-get proc 'err-file))
+ (err-buffer (or (process-get proc 'err-buffer)
+ (find-file-noselect err-file))))
+ (when err-buffer (kill-buffer err-buffer))))
+
;; This variable is used only buffer local, but it needs to be
;; declared globally first to avoid compiler warnings.
(defvar notmuch-show-process-crypto nil)
--
2.17.1 (Apple Git-112)

dme.

[Sebastian Schwarz]

Post by David Edmondson
After the changes to use `make-process', the Carbon port of emacs on
macOS (often referred to as emacs-mac or the railwaycat port) will ask
Debugger entered--Lisp error: (quit)
yes-or-no-p("Buffer \" *notmuch-stderr*-839121\" has a running process; kill it? ")
process-kill-buffer-query-function()
kill-buffer(#<buffer *notmuch-stderr*-839121>)
notmuch-start-notmuch-sentinel(#<process notmuch-search> "finished\n")

I see the same prompt whenever I call the function
notmuch-refresh-all-buffers on Ubuntu 18.04, which has GNU Emacs
25.2 and notmuch 0.26.

Post by David Edmondson
On some platforms (e.g. macOS), it is necessary to add a real sentinel
process for the error buffer used by `notmuch-start-notmuch' rather
than a no-op sentinel.

Applying your patch fixes this for me there.

[David Bremner]

Post by David Edmondson
+(defun notmuch-start-notmuch-error-sentinel (proc event)
+ (let* ((err-file (process-get proc 'err-file))
+ (err-buffer (or (process-get proc 'err-buffer)
+ (find-file-noselect err-file))))

Is the second case here (find-file-noselect) for the non-make-process
code path, or something else? It might help to have a comment.

Post by David Edmondson
+ (when err-buffer (kill-buffer err-buffer))))
+

[David Edmondson]

Post by David Bremner

Post by David Edmondson
+(defun notmuch-start-notmuch-error-sentinel (proc event)
+ (let* ((err-file (process-get proc 'err-file))
+ (err-buffer (or (process-get proc 'err-buffer)
+ (find-file-noselect err-file))))

Is the second case here (find-file-noselect) for the non-make-process
code path, or something else? It might help to have a comment.

For non-make-process. Updated patch sent.

Post by David Bremner

Post by David Edmondson
+ (when err-buffer (kill-buffer err-buffer))))
+

dme.