Re: [PATCH v2 0/1] MR6800: winegstreamer: Lock the media source critical section while using the event queue.

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/winegstreamer/media_source.c:
> -
> * erroneously return S_OK here. */
> - if (source->state == SOURCE_SHUTDOWN)
> -
>    hr = MF_E_SHUTDOWN;
> - else
> - {
> -
>    event_queue = source->event_queue;
> -
>    IMFMediaEventQueue_AddRef(event_queue);
> - }
> -
> - LeaveCriticalSection(&source->cs);
> -
> - if (SUCCEEDED(hr))
> - {
> -
>    hr = IMFMediaEventQueue_GetEvent(event_queue, flags, event);
> -
>    IMFMediaEventQueue_Release(event_queue);
> - }

I still don't see how this makes any difference? Let's assume there's an event E in the event queue prior to these calls:

Before this change we can have A3-A5 serialized with B5:

```
Thread A:                                         Thread B:
A1: IMFMediaSource_GetEvent( source )
                                                  B1: IMFMediaSource_Shutdown( source )
                                                  B2: EnterCriticalSection( &source->cs )
                                                  B3: source->state := SOURCE_SHUTDOWN
                                                  B4: IMFMediaEventQueue_Shutdown( queue )
A2: IMFMediaEventQueue_GetEvent( queue )
A3:   EnterCriticalSection( &queue->cs )
A4:   queue->is_shut_down == FALSE
A5:   LeaveCriticalSection( &queue->cs )
                                                  B5:   EnterCriticalSection( &queue->cs )
                                                  B6:   queue->is_shut_down := TRUE
                                                  B7:   LeaveCriticalSection( &queue->cs )
                                                  B8: IMFMediaEventQueue_Shutdown -> (S_OK)
                                                  B9: LeaveCriticalSection( &source->cs )
                                                  B10: IMFMediaSource_Shutdown -> S_OK
A6: IMFMediaEventQueue_GetEvent -> (S_OK, E)
```

After this change we can have A2-A5 serialized with B3, and A7-A9 serialized with B5:

```
Thread A:                                         Thread B:
A1: IMFMediaSource_GetEvent( source )
                                                  B1: IMFMediaSource_Shutdown( source )
A2: EnterCriticalSection( &source->cs )
A3: source->state != SOURCE_SHUTDOWN
A4: queue := source->queue
A5: LeaveCriticalSection( &source->cs )
                                                  B2: EnterCriticalSection( &source->cs )
                                                  B3: source->state := SOURCE_SHUTDOWN
                                                  B4: IMFMediaEventQueue_Shutdown( queue )
A6: IMFMediaEventQueue_GetEvent( queue )
A7:   EnterCriticalSection( &queue->cs )
A8:   queue->is_shut_down == FALSE
A9:   LeaveCriticalSection( &queue->cs )
                                                  B5:   EnterCriticalSection( &queue->cs )
                                                  B6:   queue->is_shut_down := TRUE
                                                  B7:   LeaveCriticalSection( &queue->cs )
                                                  B8: IMFMediaEventQueue_Shutdown -> (S_OK)
                                                  B9: LeaveCriticalSection( &source->cs )
                                                  B10: IMFMediaSource_Shutdown -> S_OK
A10: IMFMediaEventQueue_GetEvent -> (S_OK, E)
```

Makes strictly no difference in terms of possible outcome?

I think we need to determine the cause of the hang more precisely, especially if it still happens after this change because I don't think it makes any logical difference.