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

core: deadlock fix, remove empty system, relax version request #2300

Merged
merged 3 commits into from
May 14, 2024

Conversation

julianoes
Copy link
Collaborator

Three issues I found running multiple instances overnight. See commits.

I don't believe it is still required to create these fake systems.
That's because we send out heartbeats from MavsdkImpl or rather
ServerComponentImpl, and not from within SystemImpl any longer which
makes a lot more sense anyway.

This commit removes creating the fake system.

Signed-off-by: Julian Oes <julian@oes.ch>
This should fix the deadlock which can happen when a timeout happens at
the exact time when a heartbeat arrives again.

In that case one thread will call:
SystemImpl::heartbeats_timed_out() -> acquire _connection_mutex
SystemImpl::set_disconnected()
MavsdkImpl::notify_on_timeout()    -> acquire _systems_mutex

The other thread will call:
MavsdkImpl::receive_message()      -> acquire _systems_mutex
SystemImpl::set_connected()        -> acquire _connection_mutex

So both threads end up waiting for each other's mutex and dead lock.

This commit fixes the problem by moving the on_discover and on_timeout
calls outside of the scope of where the _connection_mutex is acquired.

Signed-off-by: Julian Oes <julian@oes.ch>
Instead of doing too many commands at once, we only do one at a time,
and don't immediately call ourselves again. We don't want to hold up the
plugins' enable() calls for too long.

Also, we can make the request interval slightly longer.

Signed-off-by: Julian Oes <julian@oes.ch>
Copy link

sonarcloud bot commented May 14, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@julianoes julianoes merged commit e815b8b into main May 14, 2024
28 checks passed
@julianoes julianoes deleted the pr-lockup-fixes branch May 14, 2024 07:12
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.

1 participant