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

Wait on condition variables independently on system time #4457

Merged
merged 14 commits into from
Mar 16, 2024

Conversation

AlexGuteniev
Copy link
Contributor

Fix #718

Possibly create chaos (I hope not).

I made new core header, since making <xthreads.h> non-core looked hard.

Thanks @achabense for the first step in this direction.

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner March 8, 2024 19:08
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Mar 8, 2024
@StephanTLavavej StephanTLavavej self-assigned this Mar 8, 2024
stl/inc/condition_variable Outdated Show resolved Hide resolved
stl/inc/xthreads.h Outdated Show resolved Hide resolved
stl/src/sharedmutex.cpp Outdated Show resolved Hide resolved
stl/src/sharedmutex.cpp Outdated Show resolved Hide resolved
stl/src/primitives.hpp Show resolved Hide resolved
stl/inc/__msvc_threads_core.hpp Show resolved Hide resolved
stl/src/primitives.hpp Outdated Show resolved Hide resolved
stl/inc/thread Outdated Show resolved Hide resolved
stl/src/sharedmutex.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Mar 13, 2024
@StephanTLavavej
Copy link
Member

⚠️ Note to self:

This will need the usual MSVC-internal changes to add a new header.

@AlexGuteniev
Copy link
Contributor Author

One of scary things here is locking down the _Cnd_t and _Mtx_t.
We cannot further change them after this change, as the assumption on their structure are embedded into import library.

@AlexGuteniev
Copy link
Contributor Author

One of scary things here is locking down the _Cnd_t and _Mtx_t. We cannot further change them after this change, as the assumption on their structure are embedded into import library.

The alternative could be a satellite DLL, this makes the change possible without core header extraction, but this can be more cumbersome overall.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Mar 13, 2024

I think I'm fine with that limitation - the headers (not just the import lib) already know about the mutex representation, and the condition variable representation has been boiled down to its essentials now.

Satellite DLLs are really annoying. I don't think we'll ever use them again unless we're absolutely forced to.

There's one more transformation I want to perform (I want to get rid of the forward declaration squirrelliness) but that won't affect representation.

@StephanTLavavej StephanTLavavej self-assigned this Mar 15, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 4b80c7c into microsoft:main Mar 16, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for fixing this notorious bug that we previously thought was unfixable! ⭐ 😻 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

<thread>: this_thread::sleep_until uses system time even if based on steady_clock
2 participants