On 09/16/2019 02:44 AM, Henri Verbeet wrote:
On Sun, 15 Sep 2019 at 20:09, Zebediah Figura z.figura12@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?
ἔρρωσο