Hello,

while using the emacs password-store.el package (latest version from
https://git.zx2c4.com/password-store with pass v1.7.4 on Windows/WSL2) I
found that certain commands (like password-store--run-generate,
password-store--run-remove) cause an infloop in:

(defun password-store--run (&rest args)
  "Run pass with ARGS.

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))
    output))

The problem is the `(while (not output) (sleep-for .1))` loop which
never receives any output (from the supposedly asynchronous pass process
started via `password-store--run-1').

There seems to be a race condition as e.g. stepping through the code in
edebug never caused the issue. Also, and this seems really weird, the
issue didn't happen when adding passwords to the root directory of the
password store (i.e. not in a subdirectory). As mentioned above, I'm
running emacs in WSL2 on Windows so this might have an effect on timing.

While I'm no expert in emacs process handling, I found code in other
packages which solved the 'wait for async process output' problem by
using the process object returned from `password-store--run-1' to make
sure that process output gets seen/accepted by emacs and check for
the termination of the aync process. Applying this to
`password-store--run' results in:

  (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))

which, at least in my environment, fixes the issue.

I've CCed Ian who initially added the async handling in commit 7aa17d0.
Maybe he can comment on this?

A patch with the change shown above against current master is attached.

Best Regards,
Kai

>From 7b3f8069cf1e7a462e88c676f59ffa26fbb1adc3 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 | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/contrib/emacs/password-store.el b/contrib/emacs/password-store.el
index 61c339e..98ae596 100644
--- a/contrib/emacs/password-store.el
+++ b/contrib/emacs/password-store.el
@@ -99,12 +99,14 @@ 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))))
+    (accept-process-output process .1 nil t)
+    (while (process-live-p process)
+      (accept-process-output process .1 nil t))
     output))
 
 (defun password-store--run-async (&rest args)
-- 
2.34.1

Reply via email to