frontend/nine: Fix destruction race

As we were checking the bind count after decreasing the ref count,
we could hit a situation where the worker thread decreases the bind count
just after the ref count is decreased, but before the bind count is checked
which would lead to both threads calling the resource dtor.

Cc: mesa-stable
Signed-off-by: Axel Davy <davyaxel0@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28232>
(cherry picked from commit d6044cf857)
This commit is contained in:
Axel Davy 2023-05-14 12:04:22 +02:00 committed by Eric Engestrom
parent 63873590d8
commit 7bf97678dd
3 changed files with 18 additions and 6 deletions

View file

@ -224,7 +224,7 @@
"description": "frontend/nine: Fix destruction race",
"nominated": true,
"nomination_type": 0,
"resolution": 0,
"resolution": 1,
"main_sha": null,
"because_sha": null,
"notes": null

View file

@ -48,6 +48,7 @@ NineUnknown_ctor( struct NineUnknown *This,
This->forward = false;
This->bind = 0;
}
This->has_bind_or_refs = This->bind + This->refs;
This->container = pParams->container;
This->device = pParams->device;
@ -119,6 +120,7 @@ NineUnknown_AddRef( struct NineUnknown *This )
r = p_atomic_inc_return(&This->refs);
if (r == 1) {
p_atomic_inc(&This->has_bind_or_refs);
if (This->device)
NineUnknown_AddRef(NineUnknown(This->device));
}
@ -142,9 +144,11 @@ NineUnknown_Release( struct NineUnknown *This )
if (r == 0) {
struct NineDevice9 *device = This->device;
UINT b_or_ref = p_atomic_dec_return(&This->has_bind_or_refs);
/* Containers (here with !forward) take care of item destruction */
if (!This->container && This->bind == 0) {
if (!This->container && b_or_ref == 0) {
assert(p_atomic_read(&This->bind) == 0);
This->dtor(This);
}
if (device) {
@ -166,8 +170,10 @@ NineUnknown_ReleaseWithDtorLock( struct NineUnknown *This )
if (r == 0) {
struct NineDevice9 *device = This->device;
UINT b_or_ref = p_atomic_dec_return(&This->has_bind_or_refs);
/* Containers (here with !forward) take care of item destruction */
if (!This->container && This->bind == 0) {
if (!This->container && b_or_ref == 0) {
assert(p_atomic_read(&This->bind) == 0);
NineLockGlobalMutex();
This->dtor(This);
NineUnlockGlobalMutex();

View file

@ -47,6 +47,7 @@ struct NineUnknown
int32_t refs; /* external reference count */
int32_t bind; /* internal bind count */
int32_t has_bind_or_refs; /* 0 if no ref, 1 if bind or ref, 2 if both */
bool forward; /* whether to forward references to the container */
/* container: for surfaces and volumes only.
@ -130,7 +131,7 @@ NineUnknown_FreePrivateData( struct NineUnknown *This,
static inline void
NineUnknown_Destroy( struct NineUnknown *This )
{
assert(!(This->refs | This->bind));
assert(!(This->refs | This->bind) && !This->has_bind_or_refs);
This->dtor(This);
}
@ -140,6 +141,8 @@ NineUnknown_Bind( struct NineUnknown *This )
UINT b = p_atomic_inc_return(&This->bind);
assert(b);
if (b == 1)
p_atomic_inc(&This->has_bind_or_refs);
if (b == 1 && This->forward)
NineUnknown_Bind(This->container);
@ -150,10 +153,13 @@ static inline UINT
NineUnknown_Unbind( struct NineUnknown *This )
{
UINT b = p_atomic_dec_return(&This->bind);
UINT b_or_ref = 1;
if (b == 0)
b_or_ref = p_atomic_dec_return(&This->has_bind_or_refs);
if (b == 0 && This->forward)
NineUnknown_Unbind(This->container);
else if (b == 0 && This->refs == 0 && !This->container)
else if (b_or_ref == 0 && !This->container)
This->dtor(This);
return b;
@ -173,7 +179,7 @@ NineUnknown_Detach( struct NineUnknown *This )
assert(This->container && !This->forward);
This->container = NULL;
if (!(This->refs | This->bind))
if (!(This->has_bind_or_refs))
This->dtor(This);
}