Re: master 8dcb19f 4/4: Add a unit test testing interaction between threads and processes.

[Philipp]

> Am 24.01.2021 um 16:30 schrieb Eli Zaretskii <eliz@gnu.org>:
> 
>> From: Philipp Stephani <p.stephani2@gmail.com>
>> Date: Sun, 24 Jan 2021 14:22:57 +0100
>> Cc: Philipp Stephani <phst@google.com>, Emacs developers 
>> <emacs-devel@gnu.org>
>> Right now we don't have that many good tests that test the interaction
>> between threads and processes, so the intention of this test is to
>> improve coverage in this area.
> 
> The general intent is clear and welcome; it's the details that I'm
> interested in, because I need to know which, if any, parts of the
> Windows emulation of the sub-process support need to be improved or
> fixed.

The specific scenario here is: Start a number of process and a number of 
threads, each of which waits for one of the processes; then, cause the 
processes to exit (since they run „cat“, that works by sending EOF); then, join 
all the threads.  This should work without hangs or errors.

> 
>>> There are some things the code does whose rationale I
>>> don't understand.  For example, why the call to set-process-thread:
>>> what's the purpose?
>> 
>> The test wouldn't work without it, because it calls
>> accept-process-output from a thread different from the thread in which
>> the process was created.
> 
> But set-process-thread accepts a thread argument, so you could bind
> the process to a particular thread, couldn't you?

Yes, but the threads that call `accept-process-output’ and `process-send-eof’ 
are different threads; binding the process object to one of the threads would 
cause the other call to fail.

> 
> More generally, my reading of the code is that the first thread which
> calls accept-process-output will start listening on all the file
> descriptors of all the sub-processes which were started until then,
> and all the other threads will have no descriptors left to listen on.
> Is this the situation you intended to create?

No, but I’m not convinced that that’s actually how the implementation behaves.  
Wouldn’t with the current implementation each thread wait for the file 
descriptors of one process?

>  And if so, is this an
> interesting practical use case, or are we just testing implementation
> details?

The goal here is definitely to test the observable behavior; the implementation 
should follow from that.  My interpretation of the Elisp manual is that the 
test should succeed without errors (but unfortunately the manual is still a bit 
vague on these matters).

> 
>>> Or why no-conversion in make-process?
>> 
>> Unit tests should typically test only one aspect, or a small number of
>> related aspects of the system under test. The goal of this test is not
>> to test the encoding procedures, so I turn off encoding here.
> 
> Thanks, so I now understand that the encoding stuff is not important
> (since the subprocess doesn't produce any output, it cannot have any
> effect, AFAIU).

Yes.  I’d be fine removing the :coding argument, but `no-conversion’ seems to 
be the simplest choice.

> 
>>> More
>>> generally, what is the basic idea of the test and the expectation
>>> from each thread.
>> 
>> I don't understand this question. The idea is to have a test that
>> calls accept-process-output from multiple threads to increase coverage
>> in this area.
> 
> Why is it important to test this situation, and what do we expect to
> happen in it?  Is it reasonable to have more than one thread wait for
> output from a subprocess?

The tests shouldn’t ask what’s reasonable, but what’s documented or expected.  
Surely users can write code like this, and I don’t see why that shouldn’t be 
supported.

> 
>>>> thread-join should yield.
>>> 
>>> Isn't that too late?  The processes have exited already, so what does
>>> that test?
>> 
>> Again, I don't understand this question. Each unit test is a runnable
>> example: by writing it, we document some aspect about how Emacs Lisp
>> should behave, and by running it, we can find bugs (mismatches between
>> the intended and the actual behavior) and regressions.
> 
> The question I ask myself is what to do with the deviant behavior on
> MS-Windows.  If it behaves differently because what we test here are
> the peculiarities of the semi-broken Unix mechanism known as "signals"
> and/or some details of our implementation of subprocesses on Posix
> platforms, then maybe there's no need to change anything in the
> Windows code, and just either skip this test on MS-Windows or write a
> different test which will work there.  IOW, in such a case there's no
> bug in the Windows port, it's just that it isn't bug-for-bug
> compatible with Unix.

I don’t think so.  Again, I wrote the test based on my understanding of the 
expected and promised behavior of the process and thread functions, not the 
Posix implementation.

> 
> But if what happens in this test is important in practical use cases,
> then perhaps our Windows code does need some changes in this area.
> 
> This is why I'm trying to understand the semantics of this test -- I'd
> like to figure out which implementation details are important, and
> thus should be emulated on Windows, and which are just details that
> can be different (and portable Lisp programs will have to take those
> differences into account).

The way I see it, all aspects of the test are important — their behavior should 
be guaranteed by the implementation.

> 
> For example, the test verifies that each process exited with zero
> status, but it doesn't care which thread detected that -- is this
> important?

IIUC threads don’t really „detect“ process exit.  What I’d assume is that the 
`thread-join’ calls are synchronization points — after calling `thread-join’ 
the thread in question is guaranteed to have finished, and since the thread has 
been waiting for a process to finish (using the accept-process-output loop), 
the process is also guaranteed to have finished, and this sequence of event is 
guaranteed exactly in that order.  At least that’s roughly the meaning of such 
synchronization points in other languages.  Since the process is guaranteed to 
have finished, it will necessarily need to have the right exit status stored in 
its process object, and since „cat“ finishes with a zero exit status in case 
its stdin gets closed, that must be zero.

>  Or what about delivery of SIGCHLD -- is this important
> which thread gets delivered the signal, or how many threads should the
> signal wake up?

Not as long as the observable behavior stays the same (and isn’t buggy).

>  Or is it important when and due to what reason(s)
> does each thread exit?


It’s important that each thread exits normally, i.e., due to its thread 
function body exiting normally.  It’s not important when exactly the thread 
finishes, but the observable order of events matters: first process-send-eof, 
then process exit, then thread exit, then thread-join returns.