Re: master 64f2c96 2/2: Make a process test faster.

Re: master 64f2c96 2/2: Make a process test faster.

[Glenn Morris]

> branch: master
> commit 64f2c96cbe3ba803c4026c976c425771911e29e3

>     Make a process test faster.

Hi - this makes the test fail for me on CentOS 8.3, and also for
https://hydra.nixos.org/build/134145457

Re: master 64f2c96 2/2: Make a process test faster.

[Philipp Stephani]

Am So., 3. Jan. 2021 um 18:42 Uhr schrieb Glenn Morris <rgm@gnu.org>:
>
>
> > branch: master
> > commit 64f2c96cbe3ba803c4026c976c425771911e29e3
>
> >     Make a process test faster.
>
> Hi - this makes the test fail for me on CentOS 8.3, and also for
> https://hydra.nixos.org/build/134145457

I've now tried to make this test more robust with commit
57e872ac757d7a003f4a7f132a08798c3a1a6e97.

Re: master 64f2c96 2/2: Make a process test faster.

[Philipp Stephani]

Am Mo., 4. Jan. 2021 um 22:24 Uhr schrieb Philipp Stephani
<p.stephani2@gmail.com>:
>
> Am So., 3. Jan. 2021 um 18:42 Uhr schrieb Glenn Morris <rgm@gnu.org>:
> >
> >
> > > branch: master
> > > commit 64f2c96cbe3ba803c4026c976c425771911e29e3
> >
> > >     Make a process test faster.
> >
> > Hi - this makes the test fail for me on CentOS 8.3, and also for
> > https://hydra.nixos.org/build/134145457
>
> I've now tried to make this test more robust with commit
> 57e872ac757d7a003f4a7f132a08798c3a1a6e97.


Looks like that helped: https://hydra.nixos.org/build/134431253

Re: master 64f2c96 2/2: Make a process test faster.

[Glenn Morris]

It is now unstable, ie mostly passes but sometimes fails.

Ref eg
https://hydra.nixos.org/build/134561612
https://hydra.nixos.org/build/134556213

Same seems to apply to at least:
tramp-test31-interrupt-process
https://hydra.nixos.org/build/134553525

mml-secure-en-decrypt-2
https://hydra.nixos.org/build/134547492

Re: master 64f2c96 2/2: Make a process test faster.

[Michael Albinus]

Glenn Morris <rgm@gnu.org> writes:

> Same seems to apply to at least:
> tramp-test31-interrupt-process
> https://hydra.nixos.org/build/134553525

This was is already marked as unstable on emba. So I've done it also for hydra.

Best regards, Michael.

Re: master 64f2c96 2/2: Make a process test faster.

[Philipp Stephani]

Am Fr., 8. Jan. 2021 um 02:38 Uhr schrieb Glenn Morris <rgm@gnu.org>:
>
>
> It is now unstable, ie mostly passes but sometimes fails.
>
> Ref eg
> https://hydra.nixos.org/build/134561612
> https://hydra.nixos.org/build/134556213

Interesting. I can't see why it would deadlock. There are only 4 possible cases:
1. vfork and exec both succeed. Then the code will immediately close
stdin for cat, causing it to terminate, and accept-process-output
should return immediately after one or two iterations.
2. vfork and exec both fail due to running out of file descriptors.
Then the process object doesn't exist in the first place, and we don't
try to read from it.
3. Some unexpected error (out of memory, executable suddenly vanishes,
...). That should result in a test failure, but no deadlock.
4. vfork succeeds, child setup before exec fails. That's the
interesting case, which I think is happening here: the child tries to
reopen the TTY file, but that fails because of too many open files. In
this case, since make-process waits for exec/exit, the process object
exists, but isn't alive any more. However, in that case
accept-process-output should also return immediately.

Re: master 64f2c96 2/2: Make a process test faster.

[Philipp Stephani]

Am Sa., 9. Jan. 2021 um 19:12 Uhr schrieb Philipp Stephani
<p.stephani2@gmail.com>:
>
> Am Fr., 8. Jan. 2021 um 02:38 Uhr schrieb Glenn Morris <rgm@gnu.org>:
> >
> >
> > It is now unstable, ie mostly passes but sometimes fails.
> >
> > Ref eg
> > https://hydra.nixos.org/build/134561612
> > https://hydra.nixos.org/build/134556213
>
> Interesting. I can't see why it would deadlock.

OK, I think I've found 2 bugs.
1. I've introduced one myself when writing emacs_spawn - we need to
mark processes as live before unblocking SIGCHLD, otherwise the
SIGCHLD handler would never call waitpid for them.
2. The other one happens if SIGCHLD is signaled during
wait_reading_process_output. Then wait_reading_process_output will
wait forever, since the stdout FD never gets closed and it doesn't see
the process status update in time. Not sure how to fix this - the most
elegant way would be pidfd/pdfork, but that's not available on all
systems.

Re: master 64f2c96 2/2: Make a process test faster.

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 9 Jan 2021 22:00:45 +0100
> Cc: Philipp Stephani <phst@google.com>, Emacs developers <emacs-devel@gnu.org>
> 2. The other one happens if SIGCHLD is signaled during
> wait_reading_process_output. Then wait_reading_process_output will
> wait forever, since the stdout FD never gets closed and it doesn't see
> the process status update in time.

Do you mean that wait_reading_process_output has this problem in
general, or just in this particular scenario?  If the former, I'm
surprised, as we are using this code for a very long time.  If the
latter, can you elaborate on the situation, and what does SIGCHLD have
to do with closing stdout?

Re: master 64f2c96 2/2: Make a process test faster.

[Philipp Stephani]

Am So., 10. Jan. 2021 um 17:39 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 9 Jan 2021 22:00:45 +0100
> > Cc: Philipp Stephani <phst@google.com>, Emacs developers <emacs-devel@gnu.org>
> > 2. The other one happens if SIGCHLD is signaled during
> > wait_reading_process_output. Then wait_reading_process_output will
> > wait forever, since the stdout FD never gets closed and it doesn't see
> > the process status update in time.
>
> Do you mean that wait_reading_process_output has this problem in
> general, or just in this particular scenario?  If the former, I'm
> surprised, as we are using this code for a very long time.  If the
> latter, can you elaborate on the situation, and what does SIGCHLD have
> to do with closing stdout?

I believe is a rather general problem. Anecdotally I've heard
occasionally about problems with accept-process-output deadlocking,
which might be related. See e.g. some of the other tests in
process-tests.el. AIUI, the sequence of events is as follows:
1. (accept-process-output PROC)
2. Here the process isn't finished yet. accept-process-output waits
for something to become available on stdout.
3. If the process doesn't write anything to stdout,
accept-process-output will block.
4. The process exits without having written anything.
5. Stdout is closed.
6. pselect returns, but since the process hasn't written anything,
wait_reading_process_output doesn't return.
7. Emacs receives SIGCHLD.
8. Emacs tries to notify accept-process-output, but it's too late - we
are already within the pselect call, which now hangs forever.
To test that, I added some printf statements, and indeed I saw that
wait_reading_process_output was entered, then SIGCHLD was received,
but wait_reading_process_output continued to hang.
I remedied this on the scratch/sigchild-fd branch by having the
SIGCHLD handler signal a pipe that pselect watches. That fixes the
deadlock entirely for me on GNU/Linux and macOS.
(Going forward we might want to use pidfds if available, they seem
simpler and less error-prone than signals.)

Re: master 64f2c96 2/2: Make a process test faster.

[Philipp Stephani]

Am So., 10. Jan. 2021 um 18:17 Uhr schrieb Philipp Stephani
<p.stephani2@gmail.com>:

> I remedied this on the scratch/sigchild-fd branch by having the
> SIGCHLD handler signal a pipe that pselect watches. That fixes the
> deadlock entirely for me on GNU/Linux and macOS.

After checking that this branch doesn't break the build on either
Windows, macOS, or GNU/Linux, I've pushed it to master.