From e253b2343d9fc06eb3e6576d7be8b8dcae225efc Mon Sep 17 00:00:00 2001 From: Julian Lettner Date: Wed, 23 Jun 2021 15:43:39 -0700 Subject: [PATCH 1/4] Synchronize both versions of actor_counters.swift test (cherry picked from commit 97266813899f593de3e02bfcb0d661c49f50880d) --- test/Concurrency/Runtime/actor_counters.swift | 6 ++++++ test/Sanitizers/tsan/actor_counters.swift | 10 ++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/test/Concurrency/Runtime/actor_counters.swift b/test/Concurrency/Runtime/actor_counters.swift index d483726161a54..44ed1aeaa4fd5 100644 --- a/test/Concurrency/Runtime/actor_counters.swift +++ b/test/Concurrency/Runtime/actor_counters.swift @@ -28,6 +28,12 @@ actor Counter { value = value + 1 return current } + + deinit { + for i in 0.. Date: Wed, 23 Jun 2021 16:20:06 -0700 Subject: [PATCH 2/4] Synchronize on Job address Make sure to synchronize on Job address (AsyncTasks are Jobs, but not all Jobs are AsyncTasks). (cherry picked from commit 743e56e9ca2c28d342501a2ea204e1605b75ae25) --- stdlib/public/Concurrency/Task.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/public/Concurrency/Task.cpp b/stdlib/public/Concurrency/Task.cpp index d40895c6720bd..6c00f5e26d619 100644 --- a/stdlib/public/Concurrency/Task.cpp +++ b/stdlib/public/Concurrency/Task.cpp @@ -950,7 +950,7 @@ static void resumeTaskAfterContinuation(AsyncTask *task, // Make sure TSan knows that the resume call happens-before the task // restarting. - _swift_tsan_release(task); + _swift_tsan_release(static_cast(task)); // The status should be either Pending or Awaited. If it's Awaited, // which is probably the most likely option, then we should immediately From 4180b98506e8908248797c8910ce9b237a6f2a08 Mon Sep 17 00:00:00 2001 From: Julian Lettner Date: Thu, 24 Jun 2021 18:15:58 -0700 Subject: [PATCH 3/4] Add fprintf debug output for TSan acquire/release (cherry picked from commit 864026699ddbd1cd9b6d559a2148694194a77299) --- stdlib/public/Concurrency/ThreadSanitizer.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/stdlib/public/Concurrency/ThreadSanitizer.cpp b/stdlib/public/Concurrency/ThreadSanitizer.cpp index 336c35ff8535d..b28742a3644b9 100644 --- a/stdlib/public/Concurrency/ThreadSanitizer.cpp +++ b/stdlib/public/Concurrency/ThreadSanitizer.cpp @@ -31,12 +31,18 @@ TSanFunc *tsan_acquire, *tsan_release; void swift::_swift_tsan_acquire(void *addr) { if (tsan_acquire) { tsan_acquire(addr); +#if SWIFT_TASK_PRINTF_DEBUG + fprintf(stderr, "[%lu] tsan_acquire on %p\n", _swift_get_thread_id(), addr); +#endif } } void swift::_swift_tsan_release(void *addr) { if (tsan_release) { tsan_release(addr); +#if SWIFT_TASK_PRINTF_DEBUG + fprintf(stderr, "[%lu] tsan_release on %p\n", _swift_get_thread_id(), addr); +#endif } } From 16da6390630f1ffd6bed309d1486174f2b5db617 Mon Sep 17 00:00:00 2001 From: Julian Lettner Date: Fri, 25 Jun 2021 13:29:36 -0700 Subject: [PATCH 4/4] Move TSan release edge to swift_task_enqueueGlobal() Move the TSan release edge from `swift_task_create_commonImpl()` to `swift_task_enqueueGlobalImpl()`. Task creation itself is not an event that needs synchronization, but rather that task creation "happens before" execution of that task on another thread. This edge is usually added when the task is scheduled via `swift_task_enqueue()` (which then usually calls `swift_task_enqueueGlobal()`). However, not all task scheduling goes through the `swift_task_enqueue()` funnel as some places call the more specific `swift_task_enqueueGlobal()` directly. So let's annotate this function (duplicate edges aren't harmful) to ensure we cover all schedule events, including newly-created tasks (our original problem here). rdar://78932849 (cherry picked from commit e4e941ea7120bac558eb5ca8d95681e813b4ffa9) --- stdlib/public/Concurrency/GlobalExecutor.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/stdlib/public/Concurrency/GlobalExecutor.cpp b/stdlib/public/Concurrency/GlobalExecutor.cpp index a725d3c6f13f4..d2d9923cb8b22 100644 --- a/stdlib/public/Concurrency/GlobalExecutor.cpp +++ b/stdlib/public/Concurrency/GlobalExecutor.cpp @@ -335,6 +335,8 @@ static void swift_task_enqueueGlobalImpl(Job *job) { } void swift::swift_task_enqueueGlobal(Job *job) { + _swift_tsan_release(job); + if (swift_task_enqueueGlobal_hook) swift_task_enqueueGlobal_hook(job, swift_task_enqueueGlobalImpl); else