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

fix data race and a unsigned int rollover #7278

Merged
merged 3 commits into from
Aug 12, 2024
Merged

Conversation

gulafaran
Copy link
Contributor

@gulafaran gulafaran commented Aug 10, 2024

just some minor ubsan finds, keycodeToModifier was called with a 0 keycode on mouse keys, and the switch case rolled it over on 0 - 8, no harm but unnecessery.

the watchdog is a seperate thread and hit data races in both reading the config values and reading the bools with the main thread, add a atomic bool for the main thread to wait until the watchdog has read its configvalues so it doesnt hit races in reading the config values with main thread. and change the members to atomic aswell.

also with enabling various ubsan/threadsanitizer/asan flags it gave a few false positives from the non unicode characters in hyprdebug, just change them to a unicode name.

mouse keycode is 0, and the switch case checks for 0 - 8 and rolls over,
just return early if keycode is 0.
vaxerski
vaxerski previously approved these changes Aug 10, 2024
src/debug/HyprDebugOverlay.cpp Outdated Show resolved Hide resolved
src/config/ConfigValue.hpp Outdated Show resolved Hide resolved
vaxerski
vaxerski previously approved these changes Aug 11, 2024
src/Compositor.cpp Outdated Show resolved Hide resolved
src/Compositor.cpp Show resolved Hide resolved
asan thread sanitizer reported data races in the watchdog from reading
and setting the bool variables make them std::atomic bools. also add a
atomic bool for the main thread to wait for to avoid data race when
reading the config values.
asan created false positives and didnt like this bit, so for the sake of
easier debugging rename it to something unicode.
Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amongus?

@vaxerski vaxerski merged commit 3fa6db1 into hyprwm:main Aug 12, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants