Skip to content

🍒[5.5][Concurrency] Add tsan_release edge on task creation #38113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 27, 2021

Conversation

yln
Copy link
Contributor

@yln yln commented Jun 25, 2021

Explanation: Ensure new tasks that are not immediately scheduled via swift_task_enqueue() on creation still get a tsan_release edge once they are eventually scheduled. Not all task scheduling goes through our swift_task_enqueue() funnel, some places call swift_task_enqueueGlobal() directly instead. So also add an edge there.
Reviewer: @ktoso @jckarter
Radar/SR Issue: rdar://78932849
Risk: Small.
Testing: PR testing and CI on main.
Original PR: #38074

Julian Lettner added 4 commits June 25, 2021 15:00
Make sure to synchronize on Job address (AsyncTasks are Jobs, but not
all Jobs are AsyncTasks).

(cherry picked from commit 743e56e)
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 e4e941e)
@yln yln self-assigned this Jun 25, 2021
@yln yln requested a review from a team as a code owner June 25, 2021 22:05
@yln
Copy link
Contributor Author

yln commented Jun 25, 2021

@swift-ci Please test

@yln yln requested review from ktoso and jckarter June 25, 2021 22:20
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 16da639

@yln
Copy link
Contributor Author

yln commented Jun 25, 2021

Build failed
Swift Test Linux Platform
Git Sha - 16da639

15:41:20 <unknown>:0: error: module file '/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-release/5.5/buildbot_linux/llbuild-linux-x86_64/module-cache/2NK9P9B6291O5/SwiftShims-1KKGN8N4H61WJ.pcm' is out of date and needs to be rebuilt: signature mismatch
15:41:20 <unknown>:0: note: imported by module 'SwiftOverlayShims' in '/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-release/5.5/buildbot_linux/llbuild-linux-x86_64/module-cache/2NK9P9B6291O5/SwiftOverlayShims-1KKGN8N4H61WJ.pcm'
15:41:20 /home/buildnode/jenkins/workspace/swift-PR-Linux/branch-release/5.5/llbuild/products/llbuildSwift/BuildSystemBindings.swift:17:8: error: missing required module 'SwiftOverlayShims'
15:41:20 import Glibc
15:41:20        ^

@yln yln changed the title [cherry-pick] Add tsan_release edge on task creation 🍒[5.5][Concurrency] Add tsan_release edge on task creation Jun 26, 2021
@yln
Copy link
Contributor Author

yln commented Jun 26, 2021

@swift-ci Please test Linux

@yln yln added the r5.5 label Jun 26, 2021
@yln
Copy link
Contributor Author

yln commented Jun 26, 2021

@swift-ci please nominate

@DougGregor DougGregor merged commit 2c27eb3 into release/5.5 Jun 27, 2021
@DougGregor DougGregor deleted the tsan-actor_counters.swift-cherry-pick branch June 27, 2021 05:07
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5 labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants