Skip to content
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

<memory>: common spin lock in atomic_load/store for shared_ptr can lead to priority inversion #86

Closed
BillyONeal opened this issue Sep 10, 2019 · 1 comment
Labels
duplicate This issue or pull request already exists

Comments

@BillyONeal
Copy link
Member

BillyONeal commented Sep 10, 2019

This is a feedback report from some Xbox team folks: https://developercommunity.visualstudio.com/content/problem/716238/common-spin-lock-in-stdatomic-loadstore-for-shared.html

On the Xbox OS, the system is limited to 1 thread, because the other threads on the machine are dedicated to games. This means the STL's detection for that spinlock thinks it is on a multiprocessor machine, and uses spinning behavior accordingly. However, if a low priority thread currently holds the spinlock, a higher priority thread might spin forever and deadlock the system. There's none of the usual mitigation for priority inversions and similar like the platform locks have.

We should consider using WaitOnAddress for all STL spinlock primitives on Windows 8 or later, and/or replacing some of the spinlock primitives with SRWLOCK (which does its own spinning, and in the uncontended case is indeed acquired with a single CAS) on Windows Vista or later. That would engage the platform's priority inversion mitigations (and would completely resolve the Xbox team's problems because they get to assume Windows 10 or later).

@BillyONeal BillyONeal added bug Something isn't working performance Must go faster labels Sep 10, 2019
@BillyONeal BillyONeal self-assigned this Sep 10, 2019
@StephanTLavavej StephanTLavavej changed the title <memory>: common spin lock in std::atomic_load/store for shared_ptr can lead to priority inversion <memory>: common spin lock in atomic_load/store for shared_ptr can lead to priority inversion Sep 19, 2019
@BillyONeal BillyONeal removed their assignment Oct 28, 2019
@BillyONeal
Copy link
Member Author

Closing this one because I think my writeup in #370 is better

@BillyONeal BillyONeal added duplicate This issue or pull request already exists and removed bug Something isn't working performance Must go faster labels Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

1 participant