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 #807: recv wait can deadlock on an application thread #808

Merged
merged 10 commits into from
Jul 16, 2024

Conversation

HexKitchen
Copy link
Contributor

This is a fix for #807 : recv wait can deadlock on an application thread.

See the issue description for a discussion of the root cause.

The proposed fix here operates by introducing a thread-local variable, event_count_last_seen. After entering the mutex, which ensures a stable reading of core->event_count, the thread enters the wait for event_cond only if core->event_count is equal to event_count_last_seen. In this way, we can guarantee that we won't begin waiting on event_cond at a time when the event we need has already been broadcast, which would produce the deadlock.

@oleavr
Copy link
Member

oleavr commented Jul 12, 2024

Thanks! Great catch. Will try to get this landed on Monday, in time for 16.4.4.

@oleavr
Copy link
Member

oleavr commented Jul 15, 2024

Just pushed some changes to try a slightly simpler approach, without thread-local storage. Also pushed some style fixes/tweaks. I'll have to get some sleep now, so I'll land this first thing tomorrow -- but in case you get a chance to test in the meantime please let me know how it goes :)

@oleavr oleavr merged commit 31f9661 into frida:main Jul 16, 2024
31 of 32 checks passed
@HexKitchen
Copy link
Contributor Author

Super. Agreed this is simpler and solves the deadlock, I was able to confirm in testing as well. Thx!

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