[PATCH 2/3] winegstreamer: Merge wg_parser_disconnect and wg_parser_destroy.

[Zebediah Figura]

On 9/16/21 4:00 PM, Derek Lesho wrote:
> @@ -1702,14 +1658,42 @@ static struct wg_parser * CDECL wg_wave_parser_create(void)
>   
>   static void CDECL wg_parser_destroy(struct wg_parser *parser)
>   {
> -    if (parser->bus)
> +    unsigned int i;
> +
> +    pthread_mutex_lock(&parser->mutex);
> +    parser->shutdown = true;
> +    pthread_cond_signal(&parser->read_cond);
> +    pthread_cond_wait(&parser->state_cond, &parser->mutex);

Well, you can't just call pthread_cond_wait() not in a loop like this.

The obvious course of action is to track whether we've returned false to 
the application in wg_parser_get_next_read_offset. I don't hate that 
course of action, although I don't love it either.

There's some other options here:

(2) Use a global mutex to prevent wg_parser_get_next_read_offset() from 
dereferencing a dead parser structure. This would be a decent solution 
if we wanted to be a lot more paranoid about the PE side, although I 
don't really think we do.

(3) Reference counting.

(4) Keep the disconnect/destroy split, except that we move all of the 
actual teardown to destroy, and disconnect serves only to unblock the 
read and streaming threads. We might even go so far as to get rid of 
sink_connected and just call pthread_cond_signal(), and say that 
wg_parser_get_next_read_offset() and wg_parser_stream_get_event() might 
spuriously return false. This is roughly the way libusb works, when 
using libusb_interrupt_event_handler().

I'm inclined to like (4) best, although I'm not married to it.