On 29.09.2017 12:58, Eric Engestrom wrote:
On Thursday, 2017-09-28 20:23:16 +0200, Nicolai Hähnle wrote:
On 28.09.2017 18:37, Eric Engestrom wrote:
On Thursday, 2017-09-28 16:10:51 +0000, Nicolai Hähnle wrote:
From: Nicolai Hähnle <nicolai.haeh...@amd.com>
A tempting alternative fix would be adding a lock/unlock pair in
util_queue_fence_is_signalled. However, that wouldn't actually
improve anything in the semantics of util_queue_fence_is_signalled,
while making that test much more heavy-weight. So this lock/unlock
pair in util_queue_fence_destroy for "flushing out" other threads
that may still be in util_queue_fence_signal looks like the better
fix.
Cc: mesa-sta...@lists.freedesktop.org
---
src/util/u_queue.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/util/u_queue.c b/src/util/u_queue.c
index 449da7dc9ab..82a761e8420 100644
--- a/src/util/u_queue.c
+++ b/src/util/u_queue.c
@@ -113,20 +113,33 @@ util_queue_fence_init(struct util_queue_fence *fence)
memset(fence, 0, sizeof(*fence));
(void) mtx_init(&fence->mutex, mtx_plain);
cnd_init(&fence->cond);
fence->signalled = true;
}
void
util_queue_fence_destroy(struct util_queue_fence *fence)
{
assert(fence->signalled);
+
+ /* Protect against the following race:
+ *
+ * util_queue_fence_signal (begin)
+ * fence->signalled = true;
+ * util_queue_fence_is_signalled
+ * util_queue_fence_destroy
+ * cnd_broadcast(&fence->cond); <-- use-after-free
+ * util_queue_fence_signal (end)
+ */
+ mtx_lock(&fence->mutex);
+ mtx_unlock(&fence->mutex);
How's this protecting anything?
util_queue_fence_signal locks fence->mutex. So the main part of
util_queue_fence_destroy that actually destroy stuff will be delayed until
after the end of util_queue_fence_signal.
This feels completely wrong...
I agree it's unusual, but it's pretty much explained by the commit message
IMO.
Sorry, reading this again this morning, my message feels aggressive,
which was no my intention, so apologies for that.
No hard feelings :)
I admit that it does look weird.
Cheers,
Nicolai
The change itself looks weird, but I understand how this can work.
Not sure this is the best solution, but this code isn't my area of
expertise, so I'll leave this to you :)
Cheers,
Nicolai
+
cnd_destroy(&fence->cond);
mtx_destroy(&fence->mutex);
}
/****************************************************************************
* util_queue implementation
*/
struct thread_input {
struct util_queue *queue;
--
2.11.0
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
