[6.2][Concurrency] Move tsan_release to point before task gets destroyed #82719
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ktoso
approved these changes
Jul 2, 2025
DougGregor
reviewed
Jul 2, 2025
| @@ -256,8 +256,6 @@ void swift::runJobInEstablishedExecutorContext(Job *job) { | |||
| #if SWIFT_OBJC_INTEROP | |||
| objc_autoreleasePoolPop(pool); | |||
| #endif | |||
|
|
|||
| _swift_tsan_release(job); | |||
There was a problem hiding this comment.
This operates on every kind of job, including async tasks...
| @@ -859,6 +859,8 @@ struct AsyncTask::PrivateStorage { | |||
| } | |||
| } | |||
|
|
|||
| _swift_tsan_release(task); | |||
There was a problem hiding this comment.
This only applies to async tasks.
These 'acquire' and 'release' are referring to memory ordering, rather than reference-counting acquire and release. These don't necessarily have to be balanced; though we should check that there are also appropriate 'release' and 'acquire' calls when a Task gets interrupted and resumed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
runJobInEstablishedExecutorContextcontains an erroneous_tsan_release, which can mark memory that has already been freed as a synchronisation point for other threads. This patch moves the_tsan_releaseto a point before the deallocation.Risk: Low, this change only affects thread sanitizer.
Testing: This issue was discovered by a failing debug check within TSan when applied to a large Xcode project. The debug check no longer fails with this patch applied; unfortunately any tests that would run in Swift CI would not have this debug check, as a release build of the TSan runtime is used.
Reviewed by: @ktoso
Original PR: #82158
Radar: rdar://152501929