[PATCH v2 0/2] MR882: Use atomic write for releasing threadsafe_queue spin lock.
This fixes data race in ARM/ARM64 platforms, and prevents potential memory access reordering by the compiler. -- v2: msvcp90: Use atomic write for releasing threadsafe_queue spin lock. https://gitlab.winehq.org/wine/wine/-/merge_requests/882
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This fixes data race in ARM/ARM64 platforms, and prevents potential memory access reordering by the compiler. --- dlls/concrt140/details.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/concrt140/details.c b/dlls/concrt140/details.c index e6711db6983..fb9e2a700eb 100644 --- a/dlls/concrt140/details.c +++ b/dlls/concrt140/details.c @@ -225,7 +225,7 @@ static void threadsafe_queue_push(threadsafe_queue *queue, size_t id, queue->tail = p; if(!queue->head) queue->head = p; - queue->lock = 0; + WriteRelease(&queue->lock, 0); } else { @@ -275,7 +275,7 @@ static BOOL threadsafe_queue_pop(threadsafe_queue *queue, size_t id, queue->head = p->_Next; if(!queue->head) queue->tail = NULL; - queue->lock = 0; + WriteRelease(&queue->lock, 0); /* TODO: Add exception handling */ call__Concurrent_queue_base_v4__Deallocate_page(parent, p); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/882
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This fixes data race in ARM/ARM64 platforms, and prevents potential memory access reordering by the compiler. --- dlls/msvcp90/details.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/msvcp90/details.c b/dlls/msvcp90/details.c index 767b20b2334..8df9cbe563f 100644 --- a/dlls/msvcp90/details.c +++ b/dlls/msvcp90/details.c @@ -239,7 +239,7 @@ static void threadsafe_queue_push(threadsafe_queue *queue, size_t id, queue->tail = p; if(!queue->head) queue->head = p; - queue->lock = 0; + WriteRelease(&queue->lock, 0); } else { @@ -289,7 +289,7 @@ static BOOL threadsafe_queue_pop(threadsafe_queue *queue, size_t id, queue->head = p->_Next; if(!queue->head) queue->tail = NULL; - queue->lock = 0; + WriteRelease(&queue->lock, 0); /* TODO: Add exception handling */ call__Concurrent_queue_base_v4__Deallocate_page(parent, p); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/882
Piotr Caban (@piotr) commented about dlls/msvcp90/details.c:
queue->tail = p; if(!queue->head) queue->head = p; - queue->lock = 0; + WriteRelease(&queue->lock, 0);
Isn't it better to make the `lock` member volatile? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/882#note_8796
On Mon Sep 19 08:34:24 2022 +0000, Piotr Caban wrote:
Isn't it better to make the `lock` member volatile? There is no guarantee that volatile accesses won't be reordered by the compiler. We still need a barrier here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/882#note_8817
This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/882
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Piotr Caban (＠piotr)