Async process sentinel running exclusively in main thread?

Async process sentinel running exclusively in main thread?

[Félix Baylac Jacqué]

Hi folks,

Let's start with a bit of context:

I'd like to test a function spinning up an asynchronous process call
using ERT. Sadly, at the moment (Emacs 28.1), there's no way for ERT to
cope with asynchronous calls out of the box.

You can find a clever trick allowing you to do that online though:
Rejeep's ert-async.el library. This trick is based on abusing the
accept-process-output function. This library has been written in 2014:
at the time, condition-variables were not a thing and
accept-process-output was the best option you had to block the main
thread and wait for another thread to be done.

Things changed since then, we now have better tools to manage
concurrency in Emacs! I came to think we could probably re-write this
library using condition variables.

The overall idea is to wait for the subprocess to be done by blocking
the main thread (the one running the ERT BODY test case) using a
condition variable. The subprocess would then unblock the main thread
using its sentinel.

Overall, it transforms the async subprocess call into a synchronous for
the test case.

Which bring us to the issue: this approach results in deadlocking the
main thread.

/!\ WARNING: RUNNING THIS SNIPPET WILL DEADLOCK YOUR EMACS INSTANCE!!!!

--8<---------------cut here---------------start------------->8---
(defun test-sentinel (process event)
  (with-mutex mutex
    (progn
      (message "Sentinel: running")
      (setq exit-code 1)
      (condition-notify cond-var)
      (message "Sentinel: notified"))))

(defun test-make-process ()
  (let* ((run-async-process
          (lambda ()
            (progn
              (make-process
               :name "dummy-async-subprocess"
               :buffer "ls-buf"
               :command '("ls")
               :sentinel 'test-sentinel))))
         (run-wait-process
          (lambda ()
            (progn
              (with-mutex mutex
                (while (not exit-code)
                  (message "Wait: Waiting for exit-code")
                  (condition-wait cond-var)
                  (message "Wait: notified")))))))
    (progn
      (setq exit-code nil)
      (setq mutex (make-mutex "mutex-test"))
      (setq cond-var (make-condition-variable mutex "cond-test"))
      (make-thread run-async-process)
      (funcall run-wait-process))))

(test-make-process)
--8<---------------cut here---------------end--------------->8---

This deadlock is quite surprising for an Emacs newcomer like me: the
sentinel end up never being executed. According to the make-process
documentation, the process is supposed to be attached to the thread it
originates from. In this case the run-async-process thread. The
condition-wait originating from the main thread shouldn't lock the
subprocess sentinel.

This issue confused me for quite a while. Until I noticed something
quite surprising with the following snippet:

Note: this one is safe to run :)

--8<---------------cut here---------------start------------->8---
(defun test-sentinel (process event)
  (with-mutex mutex
    (progn
      (message "Sentinel: running")
      (message (format "Sentinel: run in main thread? %s" (equal (current-thread) main-thread)))
      (setq exit-code 1)
      (condition-notify cond-var)
      (message "Sentinel: notified"))))

(defun test-make-process ()
  (let* ((run-async-process
          (lambda ()
            (progn
              (message (format "run-async-process: run in main thread? %s" (equal (current-thread) main-thread)))
              (make-process
               :name "dummy-async-subprocess"
               :buffer "ls-buf"
               :command '("ls")
               :sentinel 'test-sentinel))))
         (run-wait-process
          (lambda ()
            (progn
              (with-mutex mutex
                (while (not exit-code)
                  (message (format "Wait: run in main thread? %s" (equal (current-thread) main-thread)))
                  (message "Wait: Waiting for exit-code")
                  (condition-wait cond-var)
                  (message "Wait: notified")))))))
    (progn
      (setq exit-code nil)
      (setq mutex (make-mutex "mutex-test"))
      (setq cond-var (make-condition-variable mutex "cond-test"))
      (make-thread run-async-process)
      (make-thread run-wait-process))))

(test-make-process)
--8<---------------cut here---------------end--------------->8---

Message output on Emacs 28.1:

--8<---------------cut here---------------start------------->8---
Wait: run in main thread? nil
Wait: Waiting for exit-code
run-async-process: run in main thread? nil
Sentinel: running
Sentinel: run in main thread? t
Sentinel: notified
Wait: notified
--8<---------------cut here---------------end--------------->8---

As you can see, while the run-async-process and the run-wait-process
functions are each living in their own threads, but the sentinel ends up
running in the main thread instead of the run-wait-process one!?

Surprising. That explains the deadlock origin though.

I couldn't find any mention of the fact that the sentinel function will
exclusively run in the main thread in the documentation. Regardless of
the thread in which the process is actually executed.

Which makes me wonder:

Is this a bug or is this an expected behavior?

In case it is an expected behavior, is there any way to wait for a
subprocess to be done in the main thread using a condition variable
without deadlocking Emacs?

Thanks a lot for taking the time to read this long message :)
Félix

Re: Async process sentinel running exclusively in main thread?

[Eli Zaretskii]

> From: Félix Baylac Jacqué <felix@alternativebit.fr>
> Date: Mon, 20 Jun 2022 10:43:52 +0200
> 
> As you can see, while the run-async-process and the run-wait-process
> functions are each living in their own threads, but the sentinel ends up
> running in the main thread instead of the run-wait-process one!?
> 
> Surprising. That explains the deadlock origin though.
> 
> I couldn't find any mention of the fact that the sentinel function will
> exclusively run in the main thread in the documentation. Regardless of
> the thread in which the process is actually executed.
> 
> Which makes me wonder:
> 
> Is this a bug or is this an expected behavior?

Expected behavior (more or less).

Sentinel runs when Emacs receives SIGCHLD due to the process's demise.
And on Posix systems, signals in Emacs are always delivered to the
main thread, because doing that in a non-main thread is unsafe (the
thread could be exiting, for example).

(The "more or less" part is because we back up the SIGCHLD mechanism
by using a self-pipe that also notifies 'pselect' that the process
died, so that 'pselect' won't wait forever in some corner cases.)

> In case it is an expected behavior, is there any way to wait for a
> subprocess to be done in the main thread using a condition variable
> without deadlocking Emacs?

Make the process output something, and wait on its output?

Btw, Emacs has a way of causing a process to be dedicated to a thread
(which AFAIU you didn't do in your code).

Re: Async process sentinel running exclusively in main thread?

[Félix Baylac Jacqué]

> Sentinel runs when Emacs receives SIGCHLD due to the process's demise.
> And on Posix systems, signals in Emacs are always delivered to the
> main thread, because doing that in a non-main thread is unsafe (the
> thread could be exiting, for example).

Thanks for the explanation, it makes sense now!

> Make the process output something, and wait on its output?

Aha. It was indeed the way to go, thanks for this really useful advice!

For posterity's sake, the following snippet ended up doing the trick.
h--call-git-in-dir being a function spinning up the asynchronous process here.

--8<---------------cut here---------------start------------->8---
(defun h--tests-init-fake-git-repo (dir)
  "Create a dummy git repo at DIR.

If DIR doesn't exist, we create it first."
  (let* ((d (file-name-as-directory dir))
         (exit-code 0)
         (git-process
          (progn
            (make-directory d t)
            (h--call-git-in-dir d
                                (lambda (ec) (setq exit-code ec))
                                "init"))))
    (progn
      (unless (file-directory-p d) (make-directory d t))
      ;; ERT does not handle async processes gracefully for the time
      ;; being. Blocking and waiting for the git process to exit
      ;; before moving on.
      (while (accept-process-output git-process)))))
--8<---------------cut here---------------end--------------->8---

> Btw, Emacs has a way of causing a process to be dedicated to a thread
> (which AFAIU you didn't do in your code).

I was relying on the implicit behavior described in the section 39.9.5
of the Elisp manual:

> by default a process is locked to the thread that created it.

I just checked this assertion with:

--8<---------------cut here---------------start------------->8---
(defun test-make-process ()
  (let* ((run-async-process
          (lambda ()
            (progn
              (message (format "run-async-process thread: %s" (prin1-to-string(current-thread))))
              (make-process
               :name "dummy-async-subprocess"
               :buffer "sleep-buf"
               :command '("sleep" "5"))))))
    (progn
      (setq process (make-thread run-async-process))
      (message (format "Main thread: %s" (prin1-to-string main-thread)))
      (message (format "Process thread: %s" (prin1-to-string process))))))

(test-make-process)
--8<---------------cut here---------------end--------------->8---

Message output:

--8<---------------cut here---------------start------------->8---
Main thread: #<thread 0x9ccc60>
Process thread: #<thread 0x631bd40>
run-async-process thread: #<thread 0x631bd40>
--8<---------------cut here---------------end--------------->8---

It does seem like the process is indeed locked to the thread that created
it (i.e. not the main thread in that example).

I guess that explicitly locking the process to a particular thread via
set-process-thread wouldn't hurt and would maybe future-proof a bit more
in case that implicit assertion breaks sometime in the future.

Again, thanks a lot for this enlightening explanation and useful advice!

Re: Async process sentinel running exclusively in main thread?

[Emanuel Berg]

Félix Baylac Jacqué wrote:

> (defun h--tests-init-fake-git-repo (dir)
>   "Create a dummy git repo at DIR.
>
> If DIR doesn't exist, we create it first."
>   (let* ((d (file-name-as-directory dir))
>          (exit-code 0)

Not used ...

>          (git-process
>           (progn
>             (make-directory d t)
>             (h--call-git-in-dir d
>                                 (lambda (ec) (setq exit-code ec))
>                                 "init"))))
>     (progn

Not needed.

>       (unless (file-directory-p d) (make-directory d t))
>       ;; ERT does not handle async processes gracefully for the time
>       ;; being. Blocking and waiting for the git process to exit
>       ;; before moving on.
>       (while (accept-process-output git-process)))))

In the docstring it doesn't say the BODY of `while' is
optional, but I just tried and it works.

Here, try it yourself, it'll hang your computer:

  (while t)

Ha. Just kidding ...

> (defun test-make-process ()
>   (let* ((run-async-process
>           (lambda ()
>             (progn

Not needed.

>               (message (format "run-async-process thread: %s"
>  (prin1-to-string(current-thread))))

You don't need `message' and then `format',

  (message "%s number %d" "Elisp programmer" 1) ; "Elisp programmer number 1"

>               (make-process
>                :name "dummy-async-subprocess"
>                :buffer "sleep-buf"
>                :command '("sleep" "5"))))))
>     (progn

Not needed.

>       (setq process (make-thread run-async-process))

Free variable, first use `defvar' if it should be global.

-- 
underground experts united
https://dataswamp.org/~incal