[PATCH 1/7] wined3d: Don't call wined3d_texture_decref() in the CS thread.

Jan Sikorski jsikorski at codeweavers.com
Mon Oct 11 11:17:42 CDT 2021


> On 11 Oct 2021, at 17:40, Henri Verbeet <hverbeet at gmail.com> wrote:
> 
> On Thu, 7 Oct 2021 at 13:06, Jan Sikorski <jsikorski at codeweavers.com> wrote:
>> ---
>> dlls/wined3d/arb_program_shader.c |   2 +-
>> dlls/wined3d/glsl_shader.c        |   2 +-
>> dlls/wined3d/surface.c            |   2 +-
>> dlls/wined3d/texture.c            | 217 ++++++++++++++++--------------
>> dlls/wined3d/wined3d_private.h    |   1 +
>> 5 files changed, 117 insertions(+), 107 deletions(-)
>> 
> I'm guessing this is a consequence of patch 5/7, but this could do
> with a bit more of a commit message; at the very least for the benefit
> of future git-blame users.
> 
>> +void wined3d_texture_destroy_temporary(struct wined3d_texture *texture)
>> +{
>> +    assert(texture->resource.ref == 1);
>> +    wined3d_texture_destroy_object(texture);
>> +    wined3d_resource_free_sysmem(&texture->resource);
>> +    context_resource_released(texture->resource.device, &texture->resource);
>> +    heap_free(texture);
>> +}
>> +
> This seems fragile. We'd need to keep this in sync with the other
> cleanup path, and that path is not trivial. If this is purely about
> avoiding taking the lock on the CS thread, it would seem preferable to
> just check for that, using something similar to
> wined3d_from_cs()/wined3d_not_from_cs().

It’s not just that, adapter_destroy_texture calls wined3d_cs_destroy_object, which is wined3d_cs_emit_callback, and that races with the application thread emitting commands into the queue. Maybe that too could be made to behave differently depending on the thread and execute the callback immediately if it’s the CS, but it also seemed error prone and convoluted to me. I’ll see what more could be done to share this code.

- Jan


More information about the wine-devel mailing list