[PATCH 11/12] ddraw: cache active DCs

[Riccardo Bortolato]

The idea behind this is that only wined3d calls need to lock that mutex.

Once we come back to ddraw, we can release it.

Ciao,
Riccardo

2015-09-23 11:25 GMT+02:00 Jonas Maebe <jonas.maebe at elis.ugent.be>:
>
> Riccardo Bortolato wrote on Wed, 23 Sep 2015:
>
> Disclaimer: I'm everything but an expert on Wine internals, so I'm just
> asking in case I'm not missing something obvious.
>
>> -    surface_impl = wined3d_surface_get_parent(wined3d_surface);
>> -    *Surface = &surface_impl->IDirectDrawSurface7_iface;
>> -    IDirectDrawSurface7_AddRef(*Surface);
>> -    TRACE("Returning surface %p.\n", Surface);
>> -    return DD_OK;
>> +    wined3d_mutex_lock();
>> +    LIST_FOR_EACH_ENTRY(surface_impl, &ddraw->active_dcs, struct
>> ddraw_surface, active_dcs_entry)
>
>
> Here you get a mutex lock before iterating over the active_dcs.
>
>> +        if (surface_dc == hdc)
>> +        {
>> +            wined3d_mutex_unlock();
>> +            *Surface = &surface_impl->IDirectDrawSurface7_iface;
>> +            IDirectDrawSurface7_AddRef(*Surface);
>> +            TRACE("Returning surface %p.\n", Surface);
>> +            return DD_OK;
>> +        }
>
>
> Here you free the lock before increasing the reference count. At first
> sight, it would seem that you either don't need the lock while iterating, or
> that you should increase the reference count before releasing the lock in
> order to avoid races (unless the lock is for a completely unrelated reason)
>
>> --- a/dlls/ddraw/surface.c
>> +++ b/dlls/ddraw/surface.c
>> @@ -2176,7 +2176,9 @@ static HRESULT WINAPI
>> ddraw_surface7_GetDC(IDirectDrawSurface7 *iface, HDC *hdc)
>>              if(hdc) *hdc = NULL;
>>              return DDERR_INVALIDPARAMS;
>>
>> -        default: return hr;
>> +        default:
>> +            list_add_head(&surface->ddraw->active_dcs,
>> &surface->active_dcs_entry);
>
>
> Here you add something to the actives_dcs list without locking, so this can
> race with the iterating (unless this is implicitly synchronised in another
> way I'm not aware of).
>
>
>> @@ -2246,6 +2248,8 @@ static HRESULT WINAPI
>> ddraw_surface7_ReleaseDC(IDirectDrawSurface7 *iface, HDC h
>>          hr = ddraw_surface_update_frontbuffer(surface, NULL, FALSE);
>>      wined3d_mutex_unlock();
>>
>> +    list_remove(&surface->active_dcs_entry);
>> +
>>      return hr;
>>  }
>
>
> Similarly, here you remove an item after releasing the lock, so it can race
> with both adding and iterating (same caveat).
>
>
> Jonas