[PATCH] wined3d: Reset the CS state before unbinding the device state.

[Zebediah Figura]

On 09/16/2019 02:44 AM, Henri Verbeet wrote:
> On Sun, 15 Sep 2019 at 20:09, Zebediah Figura <z.figura12 at gmail.com> wrote:
>>
>> Otherwise the CS might later try to unbind a resource that has already been
>> freed.
>>
> How can you get there? Generally speaking destruction goes through the
> CS as well.
> 

It's been a while since I wrote the patch, so I have to remind myself
why I did...

The primary reason is that destruction itself goes through
the CS, but the CS state is still holding pointers to those resources,
so a later call to something like wined3d_cs_exec_set_index_buffer()
would try to decrease the bind_count of an already freed resource.

The specific way this manifests is that a ddraw test will call 
wined3d_device_set_index_buffer(), then later the device gets destroyed 
and calls ddraw_set_cooperative_level() as a result. That first destroys 
the swapchain, calling wined3d_device_uninit_3d(), and then restores a 
stateblock, which sets the index buffer to NULL, and causes the CS 
thread to perform the use-after-free mentioned above.

It's not a problem currently because the stateblock that ddraw is 
capturing (as well as the device->stateblock_state directly) is holding 
an extra reference to the index buffer. In my patches to move the 
stateblock tracking I didn't preserve this reference in the client-side 
primary stateblock, and so it doesn't get captured either. I could do so 
(and, if we want to get rid of wined3d_device_get_*() functions—which 
I'm assuming we want to do?—I should), but it also seems wrong to the 
naïve reader that the CS is relying on something that's not the actual 
device state to keep alive things it's referencing.

On the other hand, I also don't know why state_unbind_resources 
specifically is called there [as opposed to nothing at all, or 
state_cleanup()]. Could you by chance provide an explanation?

ἔρρωσο