[vlc-devel] [PATCH] win32: assert when VLC_CANCEL_ADDR_SET has another address already set

[Steve Lhomme]

On Thu, Jun 16, 2016 at 5:56 PM, Steve Lhomme <robux4 at videolabs.io> wrote:
> On Thu, Jun 16, 2016 at 4:05 PM, Steve Lhomme <robux4 at videolabs.io> wrote:
>> in par with the assert in VLC_CANCEL_ADDR_CLEAR but we can tell who's guilty
>
> Here is stacktrace when it asserts. We have a vlc_cond_wait() called
> inside a vlc_cond_wait(). The latter is from a static mutex, the
> former is a general/local wait condition.

This is a static VLC_STATIC_RWLOCK that uses a static mutex (uses a
cond wait internally to wait for the lock) and a static cond_wait. But
with the current code vlc_cond_wait() calls are not meant to be
reentrant. We might do the vlc_addr_wait() on the wrong level and
never receive the matching vlc_addr_broadcast().

I wonder if we can get the wrong information when calling
vlc_interrupt_get() because of that or never return because we're
waiting for a vlc_cond_broadcast() that will happen on the wrong
signal.

>     ucrtbased.dll!common_assert_to_message_box<wchar_t>(const wchar_t
> * const expression, const wchar_t * const file_name, const unsigned
> int line_number, void * const return_address) Line 351    C++
>     ucrtbased.dll!common_assert<wchar_t>(const wchar_t * const
> expression, const wchar_t * const file_name, const unsigned int
> line_number, void * const return_address) Line 387   C++
>     ucrtbased.dll!_wassert(const wchar_t * expression, const wchar_t *
> file_name, unsigned int line_number) Line 405    C++
>>   libvlccore.dll!vlc_control_cancel(int cmd, ...) Line 794    C
>     libvlccore.dll!vlc_cancel_addr_set(void * addr) Line 994    C
>     libvlccore.dll!vlc_cancel_addr_prepare(void * addr) Line 65 C
>     libvlccore.dll!vlc_cond_wait(vlc_cond_t * cond, vlc_mutex_t *
> mutex) Line 168   C
>     libvlccore.dll!vlc_mutex_lock(vlc_mutex_t * p_mutex) Line 121   C
>     libvlccore.dll!vlc_cond_wait(vlc_cond_t * cond, vlc_mutex_t *
> mutex) Line 173   C
>     libvlccore.dll!vlc_rwlock_rdlock(vlc_rwlock * lock) Line 260    C
>     libvlccore.dll!vlc_interrupt_get() Line 164 C
>     libvlccore.dll!vlc_killed() Line 251    C
>     libvlccore.dll!stream_ReadRaw(stream_t * s, void * buf, unsigned
> int len) Line 339  C
>     libvlccore.dll!stream_Read(stream_t * s, void * buf, unsigned int
> len) Line 391 C
>     librecord_plugin.dll!Read(stream_t * s, void * p_read, unsigned
> int i_read) Line 126    C
>
>> ---
>>  src/win32/thread.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/win32/thread.c b/src/win32/thread.c
>> index 963a8d7..f23be65 100644
>> --- a/src/win32/thread.c
>> +++ b/src/win32/thread.c
>> @@ -677,6 +677,7 @@ void vlc_control_cancel (int cmd, ...)
>>              void *addr = va_arg(ap, void *);
>>
>>              EnterCriticalSection(&th->wait.lock);
>> +            assert(th->wait.addr == NULL);
>>              th->wait.addr = addr;
>>              LeaveCriticalSection(&th->wait.lock);
>>              break;
>> --
>> 2.8.2
>>