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

socket2: fix events being reordered #5955

Merged
merged 8 commits into from
May 10, 2024
Merged

Conversation

underengineering
Copy link
Contributor

Describe your PR, what does it fix/add?

fixes socket2 events being reordered due to std::thread racing with other threads in postEvent while pushing multiple events.
drops connections that have too many (64) queued events

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

no

Is it ready for merging, or does it need work?

ready

@vaxerski
Copy link
Member

vaxerski commented May 8, 2024

why are you listening for incoming events? nothing should be posted to socket2.

@underengineering
Copy link
Contributor Author

why are you listening for incoming events? nothing should be posted to socket2.

Idk, it was there before

@underengineering
Copy link
Contributor Author

Actually i think it's not needed anymore, i'll remove it

src/managers/EventManager.cpp Show resolved Hide resolved
src/managers/EventManager.hpp Outdated Show resolved Hide resolved
@underengineering
Copy link
Contributor Author

By the way, i think SHyprIPCEvent::event should be renamed to name or type, and std::strings replaced with std::string_view to reduce allocations, but i am not sure if this will break compatibility with plugins (never used them)

@vaxerski
Copy link
Member

vaxerski commented May 9, 2024

doesn't really matter that much

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.

generally lgtm

src/managers/EventManager.cpp Outdated Show resolved Hide resolved
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.

lgtm thanks

@vaxerski vaxerski merged commit 37a84c5 into hyprwm:main May 10, 2024
8 of 9 checks passed
@underengineering underengineering deleted the fix-socket2 branch May 10, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants