| Message ID | 20200126212235.29546-2-cus@passwd.hu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/udp: remove setting cancel state from the TX thread | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Sun, 26 Jan 2020 at 21:22, Marton Balint <cus@passwd.hu> wrote: > > recvfrom() is not a cancellation point in pthreads-win32, see > https://sourceware.org/pthreads-win32/manual/pthread_cancel.html > > In order to be able to cancel the reader thread on Win32 properly we first > shutdown the socket then call CancelIoEx to abort pending IO. Subsequent > recvfrom() calls will fail with WSAESHUTDOWN causing the thread to exit. > > Fixes ticket #5717. > > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavformat/udp.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/libavformat/udp.c b/libavformat/udp.c > index 85c9e3a900..23c3773c64 100644 > --- a/libavformat/udp.c > +++ b/libavformat/udp.c > @@ -1069,8 +1069,17 @@ static int udp_close(URLContext *h) > if (s->thread_started) { > int ret; > // Cancel only read, as write has been signaled as success to the user > - if (h->flags & AVIO_FLAG_READ) > + if (h->flags & AVIO_FLAG_READ) { > +#ifdef _WIN32 > + /* recvfrom() is not a cancellation point for win32, so we shutdown > + * the socket and abort pending IO, subsequent recvfrom() calls > + * will fail with WSAESHUTDOWN causing the thread to exit. */ > + shutdown(s->udp_fd, SD_RECEIVE); > + CancelIoEx((HANDLE)(SOCKET)s->udp_fd, NULL); > +#else > pthread_cancel(s->circular_buffer_thread); > +#endif > + } > ret = pthread_join(s->circular_buffer_thread, NULL); > if (ret != 0) > av_log(h, AV_LOG_ERROR, "pthread_join(): %s\n", strerror(ret)); > -- > 2.16.4 This was applied as 53aa76686e7ff4f1f6625502503d7923cec8c10e, but that commit fails to compile in my mingw cross-compile build. x86_64-w64-mingw32-gcc -I. -I./ --static -DPTW32_STATIC_LIB -D_WIN32_WINNT=0x0501 -I/opt/ffbuild/include -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U__STRICT_ANSI__ -D__USE_MINGW_ANSI_STDIO=1 -D__printf__=__gnu_printf__ -D_WIN32_WINNT=0x0600 -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -DPIC -DOPJ_STATIC -DZLIB_CONST -DHAVE_AV_CONFIG_H -DBUILDING_avformat --static -DPTW32_STATIC_LIB -D_WIN32_WINNT=0x0501 -I/opt/ffbuild/include --static -DPTW32_STATIC_LIB -D_WIN32_WINNT=0x0501 -std=c11 -fomit-frame-pointer -pthread -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include/openjpeg-2.3 -I/opt/ffbuild/include/opus -I/opt/ffbuild/include/opus -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include -DLIBXML_STATIC -I/opt/ffbuild/include/libxml2 -I/opt/ffbuild/include -g -Wdeclaration-after-statement -Wall -Wdisabled-optimization -Wpointer-arith -Wredundant-decls -Wwrite-strings -Wtype-limits -Wundef -Wmissing-prototypes -Wno-pointer-to-int-cast -Wstrict-prototypes -Wempty-body -Wno-parentheses -Wno-switch -Wno-format-zero-length -Wno-pointer-sign -Wno-char-subscripts -O3 -fno-math-errno -fno-signed-zeros -fno-tree-vectorize -Werror=format-security -Werror=implicit-function-declaration -Werror=missing-prototypes -Werror=return-type -Werror=vla -Wformat -fdiagnostics-color=auto -Wno-maybe-uninitialized -MMD -MF libavformat/udp.d -MT libavformat/udp.o -c -o libavformat/udp.o libavformat/udp.c In file included from libavformat/udp.c:65:0: /opt/ffbuild/include/pthread.h:108:0: warning: "PTW32_LEVEL" redefined #define PTW32_LEVEL PTW32_LEVEL_MAX ^ /opt/ffbuild/include/pthread.h:95:0: note: this is the location of the previous definition #define PTW32_LEVEL 1 ^ In file included from /opt/ffbuild/include/pthread.h:299:0, from libavformat/udp.c:65: /opt/ffbuild/include/sched.h:64:0: warning: "PTW32_SCHED_LEVEL" redefined #define PTW32_SCHED_LEVEL PTW32_SCHED_LEVEL_MAX ^ /opt/ffbuild/include/sched.h:51:0: note: this is the location of the previous definition #define PTW32_SCHED_LEVEL 1 ^ libavformat/udp.c: In function 'udp_close': libavformat/udp.c:1078:13: error: implicit declaration of function 'CancelIoEx' [-Werror=implicit-function-declaration] CancelIoEx((HANDLE)(SOCKET)s->udp_fd, NULL); ^ cc1: some warnings being treated as errors ffbuild/common.mak:59: recipe for target 'libavformat/udp.o' failed make: *** [libavformat/udp.o] Error 1 > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Mon, 3 Feb 2020, Mark Himsley wrote: > On Sun, 26 Jan 2020 at 21:22, Marton Balint <cus@passwd.hu> wrote: >> >> recvfrom() is not a cancellation point in pthreads-win32, see >> https://sourceware.org/pthreads-win32/manual/pthread_cancel.html >> >> In order to be able to cancel the reader thread on Win32 properly we first >> shutdown the socket then call CancelIoEx to abort pending IO. Subsequent >> recvfrom() calls will fail with WSAESHUTDOWN causing the thread to exit. >> >> Fixes ticket #5717. >> >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavformat/udp.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/libavformat/udp.c b/libavformat/udp.c >> index 85c9e3a900..23c3773c64 100644 >> --- a/libavformat/udp.c >> +++ b/libavformat/udp.c >> @@ -1069,8 +1069,17 @@ static int udp_close(URLContext *h) >> if (s->thread_started) { >> int ret; >> // Cancel only read, as write has been signaled as success to the user >> - if (h->flags & AVIO_FLAG_READ) >> + if (h->flags & AVIO_FLAG_READ) { >> +#ifdef _WIN32 >> + /* recvfrom() is not a cancellation point for win32, so we shutdown >> + * the socket and abort pending IO, subsequent recvfrom() calls >> + * will fail with WSAESHUTDOWN causing the thread to exit. */ >> + shutdown(s->udp_fd, SD_RECEIVE); >> + CancelIoEx((HANDLE)(SOCKET)s->udp_fd, NULL); >> +#else >> pthread_cancel(s->circular_buffer_thread); >> +#endif >> + } >> ret = pthread_join(s->circular_buffer_thread, NULL); >> if (ret != 0) >> av_log(h, AV_LOG_ERROR, "pthread_join(): %s\n", strerror(ret)); >> -- >> 2.16.4 > > This was applied as 53aa76686e7ff4f1f6625502503d7923cec8c10e, but that > commit fails to compile in my mingw cross-compile build. > > > x86_64-w64-mingw32-gcc -I. -I./ --static -DPTW32_STATIC_LIB > -D_WIN32_WINNT=0x0501 -I/opt/ffbuild/include -D_ISOC99_SOURCE It looks like you are still building for Windows XP, but that is no longer supported. _WIN32_WINNT should be at least 0x0600. Regards, Marton > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U__STRICT_ANSI__ > -D__USE_MINGW_ANSI_STDIO=1 -D__printf__=__gnu_printf__ > -D_WIN32_WINNT=0x0600 -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 > -DPIC -DOPJ_STATIC -DZLIB_CONST -DHAVE_AV_CONFIG_H -DBUILDING_avformat > --static -DPTW32_STATIC_LIB -D_WIN32_WINNT=0x0501 > -I/opt/ffbuild/include --static -DPTW32_STATIC_LIB > -D_WIN32_WINNT=0x0501 -std=c11 -fomit-frame-pointer -pthread > -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include > -I/opt/ffbuild/include/openjpeg-2.3 -I/opt/ffbuild/include/opus > -I/opt/ffbuild/include/opus -I/opt/ffbuild/include > -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include > -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include > -I/opt/ffbuild/include -I/opt/ffbuild/include -DLIBXML_STATIC > -I/opt/ffbuild/include/libxml2 -I/opt/ffbuild/include -g > -Wdeclaration-after-statement -Wall -Wdisabled-optimization > -Wpointer-arith -Wredundant-decls -Wwrite-strings -Wtype-limits > -Wundef -Wmissing-prototypes -Wno-pointer-to-int-cast > -Wstrict-prototypes -Wempty-body -Wno-parentheses -Wno-switch > -Wno-format-zero-length -Wno-pointer-sign -Wno-char-subscripts -O3 > -fno-math-errno -fno-signed-zeros -fno-tree-vectorize > -Werror=format-security -Werror=implicit-function-declaration > -Werror=missing-prototypes -Werror=return-type -Werror=vla -Wformat > -fdiagnostics-color=auto -Wno-maybe-uninitialized -MMD -MF > libavformat/udp.d -MT libavformat/udp.o -c -o libavformat/udp.o > libavformat/udp.c > In file included from libavformat/udp.c:65:0: > /opt/ffbuild/include/pthread.h:108:0: warning: "PTW32_LEVEL" redefined > #define PTW32_LEVEL PTW32_LEVEL_MAX > ^ > /opt/ffbuild/include/pthread.h:95:0: note: this is the location of the > previous definition > #define PTW32_LEVEL 1 > ^ > In file included from /opt/ffbuild/include/pthread.h:299:0, > from libavformat/udp.c:65: > /opt/ffbuild/include/sched.h:64:0: warning: "PTW32_SCHED_LEVEL" redefined > #define PTW32_SCHED_LEVEL PTW32_SCHED_LEVEL_MAX > ^ > /opt/ffbuild/include/sched.h:51:0: note: this is the location of the > previous definition > #define PTW32_SCHED_LEVEL 1 > ^ > libavformat/udp.c: In function 'udp_close': > libavformat/udp.c:1078:13: error: implicit declaration of function > 'CancelIoEx' [-Werror=implicit-function-declaration] > CancelIoEx((HANDLE)(SOCKET)s->udp_fd, NULL); > ^ > cc1: some warnings being treated as errors > ffbuild/common.mak:59: recipe for target 'libavformat/udp.o' failed > make: *** [libavformat/udp.o] Error 1 > > > > >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > -- > Mark Himsley > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Mon, 3 Feb 2020 at 17:19, Marton Balint <cus@passwd.hu> wrote: > > > > On Mon, 3 Feb 2020, Mark Himsley wrote: > > > On Sun, 26 Jan 2020 at 21:22, Marton Balint <cus@passwd.hu> wrote: > >> > >> recvfrom() is not a cancellation point in pthreads-win32, see > >> https://sourceware.org/pthreads-win32/manual/pthread_cancel.html > >> > >> In order to be able to cancel the reader thread on Win32 properly we first > >> shutdown the socket then call CancelIoEx to abort pending IO. Subsequent > >> recvfrom() calls will fail with WSAESHUTDOWN causing the thread to exit. > >> > >> Fixes ticket #5717. > >> > >> Signed-off-by: Marton Balint <cus@passwd.hu> > >> --- > >> libavformat/udp.c | 11 ++++++++++- > >> 1 file changed, 10 insertions(+), 1 deletion(-) > >> > >> diff --git a/libavformat/udp.c b/libavformat/udp.c > >> index 85c9e3a900..23c3773c64 100644 > >> --- a/libavformat/udp.c > >> +++ b/libavformat/udp.c > >> @@ -1069,8 +1069,17 @@ static int udp_close(URLContext *h) > >> if (s->thread_started) { > >> int ret; > >> // Cancel only read, as write has been signaled as success to the user > >> - if (h->flags & AVIO_FLAG_READ) > >> + if (h->flags & AVIO_FLAG_READ) { > >> +#ifdef _WIN32 > >> + /* recvfrom() is not a cancellation point for win32, so we shutdown > >> + * the socket and abort pending IO, subsequent recvfrom() calls > >> + * will fail with WSAESHUTDOWN causing the thread to exit. */ > >> + shutdown(s->udp_fd, SD_RECEIVE); > >> + CancelIoEx((HANDLE)(SOCKET)s->udp_fd, NULL); > >> +#else > >> pthread_cancel(s->circular_buffer_thread); > >> +#endif > >> + } > >> ret = pthread_join(s->circular_buffer_thread, NULL); > >> if (ret != 0) > >> av_log(h, AV_LOG_ERROR, "pthread_join(): %s\n", strerror(ret)); > >> -- > >> 2.16.4 > > > > This was applied as 53aa76686e7ff4f1f6625502503d7923cec8c10e, but that > > commit fails to compile in my mingw cross-compile build. > > > > > > x86_64-w64-mingw32-gcc -I. -I./ --static -DPTW32_STATIC_LIB > > -D_WIN32_WINNT=0x0501 -I/opt/ffbuild/include -D_ISOC99_SOURCE > > It looks like you are still building for Windows XP, but that is no longer > supported. _WIN32_WINNT should be at least 0x0600. Thanks Marton. I'll remember, Windows Vista and above only. Thanks - I'll update my build script. > Regards, > Marton > > > > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U__STRICT_ANSI__ > > -D__USE_MINGW_ANSI_STDIO=1 -D__printf__=__gnu_printf__ > > -D_WIN32_WINNT=0x0600 -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 > > -DPIC -DOPJ_STATIC -DZLIB_CONST -DHAVE_AV_CONFIG_H -DBUILDING_avformat > > --static -DPTW32_STATIC_LIB -D_WIN32_WINNT=0x0501 > > -I/opt/ffbuild/include --static -DPTW32_STATIC_LIB > > -D_WIN32_WINNT=0x0501 -std=c11 -fomit-frame-pointer -pthread > > -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include > > -I/opt/ffbuild/include/openjpeg-2.3 -I/opt/ffbuild/include/opus > > -I/opt/ffbuild/include/opus -I/opt/ffbuild/include > > -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include > > -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include > > -I/opt/ffbuild/include -I/opt/ffbuild/include -DLIBXML_STATIC > > -I/opt/ffbuild/include/libxml2 -I/opt/ffbuild/include -g > > -Wdeclaration-after-statement -Wall -Wdisabled-optimization > > -Wpointer-arith -Wredundant-decls -Wwrite-strings -Wtype-limits > > -Wundef -Wmissing-prototypes -Wno-pointer-to-int-cast > > -Wstrict-prototypes -Wempty-body -Wno-parentheses -Wno-switch > > -Wno-format-zero-length -Wno-pointer-sign -Wno-char-subscripts -O3 > > -fno-math-errno -fno-signed-zeros -fno-tree-vectorize > > -Werror=format-security -Werror=implicit-function-declaration > > -Werror=missing-prototypes -Werror=return-type -Werror=vla -Wformat > > -fdiagnostics-color=auto -Wno-maybe-uninitialized -MMD -MF > > libavformat/udp.d -MT libavformat/udp.o -c -o libavformat/udp.o > > libavformat/udp.c > > In file included from libavformat/udp.c:65:0: > > /opt/ffbuild/include/pthread.h:108:0: warning: "PTW32_LEVEL" redefined > > #define PTW32_LEVEL PTW32_LEVEL_MAX > > ^ > > /opt/ffbuild/include/pthread.h:95:0: note: this is the location of the > > previous definition > > #define PTW32_LEVEL 1 > > ^ > > In file included from /opt/ffbuild/include/pthread.h:299:0, > > from libavformat/udp.c:65: > > /opt/ffbuild/include/sched.h:64:0: warning: "PTW32_SCHED_LEVEL" redefined > > #define PTW32_SCHED_LEVEL PTW32_SCHED_LEVEL_MAX > > ^ > > /opt/ffbuild/include/sched.h:51:0: note: this is the location of the > > previous definition > > #define PTW32_SCHED_LEVEL 1 > > ^ > > libavformat/udp.c: In function 'udp_close': > > libavformat/udp.c:1078:13: error: implicit declaration of function > > 'CancelIoEx' [-Werror=implicit-function-declaration] > > CancelIoEx((HANDLE)(SOCKET)s->udp_fd, NULL); > > ^ > > cc1: some warnings being treated as errors > > ffbuild/common.mak:59: recipe for target 'libavformat/udp.o' failed > > make: *** [libavformat/udp.o] Error 1 > > > > > > > > -- > > Mark Himsley
diff --git a/libavformat/udp.c b/libavformat/udp.c index 85c9e3a900..23c3773c64 100644 --- a/libavformat/udp.c +++ b/libavformat/udp.c @@ -1069,8 +1069,17 @@ static int udp_close(URLContext *h) if (s->thread_started) { int ret; // Cancel only read, as write has been signaled as success to the user - if (h->flags & AVIO_FLAG_READ) + if (h->flags & AVIO_FLAG_READ) { +#ifdef _WIN32 + /* recvfrom() is not a cancellation point for win32, so we shutdown + * the socket and abort pending IO, subsequent recvfrom() calls + * will fail with WSAESHUTDOWN causing the thread to exit. */ + shutdown(s->udp_fd, SD_RECEIVE); + CancelIoEx((HANDLE)(SOCKET)s->udp_fd, NULL); +#else pthread_cancel(s->circular_buffer_thread); +#endif + } ret = pthread_join(s->circular_buffer_thread, NULL); if (ret != 0) av_log(h, AV_LOG_ERROR, "pthread_join(): %s\n", strerror(ret));
recvfrom() is not a cancellation point in pthreads-win32, see https://sourceware.org/pthreads-win32/manual/pthread_cancel.html In order to be able to cancel the reader thread on Win32 properly we first shutdown the socket then call CancelIoEx to abort pending IO. Subsequent recvfrom() calls will fail with WSAESHUTDOWN causing the thread to exit. Fixes ticket #5717. Signed-off-by: Marton Balint <cus@passwd.hu> --- libavformat/udp.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)