[FFmpeg-devel] [PATCH 3/3] avformat/udp: Enable FIFO when using windows sockets.

[Hendrik Leppkes]

On Wed, Dec 7, 2016 at 12:25 PM, Matt Oliver <protogonoi at gmail.com> wrote:
> On 7 December 2016 at 21:19, Mark Thompson <sw at jkqxz.net> wrote:
>
>> On 07/12/16 06:05, Matt Oliver wrote:
>> > Signed-off-by: Matt Oliver <protogonoi at gmail.com>
>> > ---
>> >  libavformat/udp.c | 19 +++++++++++++++++--
>> >  1 file changed, 17 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/libavformat/udp.c b/libavformat/udp.c
>> > index f8c861d..0e4766f 100644
>> > --- a/libavformat/udp.c
>> > +++ b/libavformat/udp.c
>> > @@ -64,6 +64,14 @@
>> >  #define HAVE_PTHREAD_CANCEL 0
>> >  #endif
>> >
>> > +#if !HAVE_PTHREAD_CANCEL && (HAVE_THREADS && HAVE_WINSOCK2_H)
>> > +/* Winsock2 recv function can be unblocked by shutting down and closing
>> > the socket */
>>
>> This seems dubious to me.  Can you explain how this can work reliably on
>> Windows?
>>
>> To offer some context, the reason that POSIX states that close() at the
>> same time as any other operation is undefined is because it is impossible
>> to avoid the following race:
>>
>> Thread 1:
>>   Load the file descriptor
>>   Enter the recv() call in the standard library
>>   Get preempted just before entering the system call
>> Thread 2:
>>   Call close() on the file descriptor
>>   Finish closing, the file descriptor is now invalid and can be reused
>> Thread 3 (could be thread 2 again, or a hidden part of the implementation):
>>   Make a new file (with open() or similar)
>>   Get given the same file descriptor, reused
>> Thread 1:
>>   Start running again
>>   Actually make the recv() system call, which now refers to thread 3's file
>>   Data loss/crash/other badness
>>
>> Since there is no way to determine that a thread is actually inside the
>> system call rather than preempted immediately before it there is no way in
>> POSIX to avoid the race.  An alternative implementation seeking to avoid
>> this issue could perhaps supply such a function to determine the blocked
>> state of another thread, or maybe file descriptors could be unique forever,
>> but neither of these seems to apply in this case.
>>
>
> Well to be honest this patch is actually based on a suggestion from Hendrik
> in another thread so he maybe the better person to ask about that. I can
> however state that it is a commonly used Windows winsock programming
> technique as closesocket behaves differently on windows than it does on
> linux (the above is a very bad idea on linux). Calling closesocket on
> windows causes any existing recv calls in other threads to instantly return
> with an error code. But im not sure how it would handle the specific case
> set above.

I'm not sure you can safely avoid that in this design when dealing
with a fully generic library and no information or control whats going
on in other threads.
The "proper" win32 way of doing non-polling IO is using the Overlapped
IO functions, which runs async and signals event objects when
something happens - but that would practically require re-implementing
the entire worker thread for win32.

> An alternative would be to then combine it with win32s ability to terminate
> threads.

Terminating threads is never a good idea.

- Hendrik