[FFmpeg-devel] [PATCH] libavformat/UDP Protocol : seg fault on closing

[Laurent BRULET]

Le 13/01/2012 20:28, Michael Niedermayer a écrit :
> On Fri, Jan 13, 2012 at 07:15:46PM +0100, Laurent BRULET wrote:
>> Hi ,
>>
>> The following patch fixes ticket #915 (
>> https://ffmpeg.org/trac/ffmpeg/ticket/915)
>>
>> -- Laurent
>>
>> --- diff --git a/libavformat/udp.c b/libavformat/udp.c index
>> cdcd136..c823b69 100644 --- a/libavformat/udp.c +++
>> b/libavformat/udp.c @@ -71,6 +71,7 @@ typedef struct { pthread_t
>> circular_buffer_thread; pthread_mutex_t mutex; pthread_cond_t
>> cond;
>
>> + int exit_thread;
>
> this probably needs volatile

Yes, you are right.
>
>
>
>> #endif uint8_t tmp[UDP_MAX_PKT_SIZE+4]; int remaining_in_dg; @@
>> -327,7 +328,7 @@ static void *circular_buffer_task( void
>> *_URLContext) fd_set rfds; struct timeval tv;
>>
>> - for(;;) { + while(!s->exit_thread) { int left; int ret; int
>> len;
>
>> @@ -529,6 +530,7 @@ static int udp_open(URLContext *h, const char
>> *uri, int flags)
>
> it seems the patch was linewraped
>
>
>
>> s->fifo = av_fifo_alloc(s->circular_buffer_size);
>> pthread_mutex_init(&s->mutex, NULL); pthread_cond_init(&s->cond,
>> NULL); + s->exit_thread = 0; if
>> (pthread_create(&s->circular_buffer_thread, NULL,
>
> should not be needed

Ok, UDPContext is zero allocated.
>
>
>> circular_buffer_task, h)) { av_log(h, AV_LOG_ERROR, "pthread_create
>> failed\n"); goto fail; @@ -617,12 +619,18 @@ static int
>> udp_write(URLContext *h, const uint8_t *buf, int size) static int
>> udp_close(URLContext *h) { UDPContext *s = h->priv_data; + int
>> ret;
>>
>> if (s->is_multicast && (h->flags & AVIO_FLAG_READ))
>> udp_leave_multicast_group(s->udp_fd, (struct sockaddr
>> *)&s->dest_addr); closesocket(s->udp_fd); av_fifo_free(s->fifo);
>> #if HAVE_PTHREADS + s->exit_thread = 1;
>
>> + ret = pthread_join(s->circular_buffer_thread, NULL);
>
> Is this safe in case the thread has not been started at all?

I think it is safe because, if the thread cannot start, then udp_open
returns with an error. And in ffurl_connect, in case of error
URLContext.is_connected is not set to 1. However, in ffurl_close,
udp_close is only called if URLContext.is_connect equals to 1.
I will send a new patch (a not linewrapped one, I hope)

--
Laurent