[FFmpeg-devel] [PATCH 2/2] avformat/udp: Replace use of pthread_cancel.

[Hendrik Leppkes]

On Fri, Dec 2, 2016 at 9:22 AM, Matt Oliver <protogonoi at gmail.com> wrote:
> ---
>  configure         |  6 ------
>  libavformat/udp.c | 48 +++++++++++++++++++-----------------------------
>  2 files changed, 19 insertions(+), 35 deletions(-)
>
> diff --git a/configure b/configure
> index b5bfad6..cec94c4 100755
> --- a/configure
> +++ b/configure
> @@ -1934,7 +1934,6 @@ SYSTEM_FUNCS="
>      nanosleep
>      PeekNamedPipe
>      posix_memalign
> -    pthread_cancel
>      sched_getaffinity
>      SetConsoleTextAttribute
>      SetConsoleCtrlHandler
> @@ -5623,11 +5622,6 @@ if ! disabled pthreads && ! enabled w32threads && !
> enabled os2threads; then
>      check_code cc "pthread.h" "static pthread_mutex_t atomic_lock =
> PTHREAD_MUTEX_INITIALIZER" || disable pthreads
>  fi
>
> -
> -if enabled pthreads; then
> -  check_func pthread_cancel
> -fi
> -
>  enabled pthreads &&
>      check_builtin sem_timedwait semaphore.h "sem_t *s; sem_init(s,0,0);
> sem_timedwait(s,0); sem_destroy(s)"
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 3835f98..857a979 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -60,12 +60,8 @@
>  #define IPPROTO_UDPLITE                                  136
>  #endif
>
> -#if HAVE_PTHREAD_CANCEL
> -#include <pthread.h>
> -#endif
> -
> -#ifndef HAVE_PTHREAD_CANCEL
> -#define HAVE_PTHREAD_CANCEL 0
> +#if HAVE_THREADS
> +#include "libavutil/thread.h"
>  #endif
>
>  #ifndef IPV6_ADD_MEMBERSHIP
> @@ -100,7 +96,7 @@ typedef struct UDPContext {
>      int64_t bitrate; /* number of bits to send per second */
>      int64_t burst_bits;
>      int close_req;
> -#if HAVE_PTHREAD_CANCEL
> +#if HAVE_THREADS
>      pthread_t circular_buffer_thread;
>      pthread_mutex_t mutex;
>      pthread_cond_t cond;
> @@ -495,14 +491,13 @@ static int udp_get_file_handle(URLContext *h)
>      return s->udp_fd;
>  }
>
> -#if HAVE_PTHREAD_CANCEL
> +#if HAVE_THREADS
>  static void *circular_buffer_task_rx( void *_URLContext)
>  {
>      URLContext *h = _URLContext;
>      UDPContext *s = h->priv_data;
>      int old_cancelstate;
>
> -    pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelstate);
>      pthread_mutex_lock(&s->mutex);
>      if (ff_socket_nonblock(s->udp_fd, 0) < 0) {
>          av_log(h, AV_LOG_ERROR, "Failed to set blocking mode");
> @@ -511,14 +506,11 @@ static void *circular_buffer_task_rx( void
> *_URLContext)
>      }
>      while(1) {
>          int len;
> +        if (s->close_req)
> +            goto end;
>
>          pthread_mutex_unlock(&s->mutex);
> -        /* Blocking operations are always cancellation points;
> -           see "General Information" / "Thread Cancelation Overview"
> -           in Single Unix. */
> -        pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_cancelstate);
>          len = recv(s->udp_fd, s->tmp+4, sizeof(s->tmp)-4, 0);
> -        pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &old_cancelstate);

pthread_cancel can unblock operations like these on Linux/Unix, so I
don't think some manual logic is going to solve the same problem.

On Windows this happens to work because closing the socket on another
thread unblocks any calls to it. I have had a local patch for years
that just defines pthread_cancel/setcancelstate to do nothing there.

- Hendrik