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

[Henri Verbeet]

On Thu, 19 Sep 2019 at 08:34, Zebediah Figura <z.figura12 at gmail.com> wrote:
> 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.
>
That makes sense.

> 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?
>
I'm not sure there's an especially good reason. In fact, when you add
the wined3d_cs_emit_reset_state() call, you may need state_cleanup()
instead of state_unbind_resources(). The code in question is fairly
messy and in need of some cleanups and restructuring. You've probably
also noticed that there's a fair amount of duplication with e.g.
wined3d_device_reset(). Unfortunately it's also fairly sensitive code.
Things need to be torn down in the correct order, since e.g. context
acquisition depends on having a swapchain, and depending on the D3D
version and the application, either the device or the swapchain may be
released first.