Actually, checking the elisp reference manual provided some more
insight:
If a connection from a process contains buffered data,
‘accept-process-output’ can return non-‘nil’ even after the process has
exited. Therefore, although the following loop:
;; This loop contains a bug.
(while (process-live-p process)
(accept-process-output process))
will often read all output from PROCESS, it has a race condition and can
miss some output if ‘process-live-p’ returns ‘nil’ while the connection
still contains data. Better is to write the loop like this:
(while (accept-process-output process))
So instead of:
> (let ((output nil)
> (slept-for 0)
> (process (apply #'password-store--run-1
> (lambda (password)
> (setq output password))
> (delq nil args))))
> (accept-process-output process .1 nil t)
> (while (process-live-p process)
> (accept-process-output process .1 nil t))
> output))
it should probably better be:
(let ((output nil)
(slept-for 0)
(process (apply #'password-store--run-1
(lambda (password)
(setq output password))
(delq nil args))))
;; wait for output or process termination (max. wait time: 5s)
(while (accept-process-output process 5.0))
output))
I added a timeout which terminates `accept-process-output' after 5s just
to make sure that a non-responsive pass process doesn't block emacs.
It's debatable if this is needed or even makes any sense.
An updated patch is attached.
Best Regards,
Kai
>From e5c00971f4907ae077be41bf8a28ce41e8992c52 Mon Sep 17 00:00:00 2001
From: Kai Tetzlaff <[email protected]>
Date: Sun, 27 Feb 2022 08:55:52 +0100
Subject: [PATCH] Fix infloop in password-store--run
For certain commands (like password-store--run-generate,
password-store--run-remove) with password entries in sub-directories,
password-store--run would infloop. The output variable never received any
actual output. This seems to be a timing issue as e.g. stepping through the
code in edebug never caused the issue.
This commit changes the check for the finished pass process and also inserts
calls to accept-process-output in order to guarantee that output really gets
processed while the loop is running.
---
contrib/emacs/password-store.el | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/contrib/emacs/password-store.el b/contrib/emacs/password-store.el
index 61c339e..1515e0e 100644
--- a/contrib/emacs/password-store.el
+++ b/contrib/emacs/password-store.el
@@ -99,12 +99,13 @@ or outputs error message on failure."
Nil arguments are ignored. Returns the output on success, or
outputs error message on failure."
(let ((output nil)
- (slept-for 0))
- (apply #'password-store--run-1 (lambda (password)
- (setq output password))
- (delq nil args))
- (while (not output)
- (sleep-for .1))
+ (slept-for 0)
+ (process (apply #'password-store--run-1
+ (lambda (password)
+ (setq output password))
+ (delq nil args))))
+ ;; wait for output or process termination (max. wait time: 5s)
+ (while (accept-process-output process 5.0))
output))
(defun password-store--run-async (&rest args)
--
2.34.1
