Skip to content
This repository was archived by the owner on Jan 6, 2025. It is now read-only.

Fix more potential deadlock scenarios #125

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented Mar 25, 2024

While we previously already fixed some potential deadlock issues, some more remained in the LSPS1/LSPS2 implementations due to holding the peer locks when enqueuing request/response messages as the latter would trigger PeerManager::process_events could result in circular waits. Here, we make sure to always drop the locks first, before enqueuing request/response messages.

tnull added 7 commits March 25, 2024 10:43
.. as tiny helpers like this just tend to obfuscate what's going on.
.. as tiny helpers like this just tend to obfuscate what's going on.
.. as tiny helpers like this just tend to obfuscate what's going on.
.. which avoids a potential deadlock
.. which avoids a potential deadlock
.. which avoids a potential deadlock
.. which avoids a potential deadlock
@tnull tnull force-pushed the 2024-03-fix-deadlock branch from f9d8dd5 to 6b7b875 Compare March 25, 2024 11:12
Copy link
Contributor

@wvanlint wvanlint left a comment

Choose a reason for hiding this comment

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

LGTM

I was wondering whether we can extract enqueueing the message and not repeat it, but that might only be easy for the handle_* methods.

@tnull
Copy link
Collaborator Author

tnull commented Mar 26, 2024

LGTM

I was wondering whether we can extract enqueueing the message and not repeat it, but that might only be easy for the handle_* methods.

Yeah, we could eventually consider a larger refactor to some structure that would prohibit us from ever holding the lock while enqueueing. This might be a good idea, but if we do that however, I'd like to share it across LSPS1/2 at least, which might require some more prefactors to make them more uniform.

Landing this for now.

@tnull tnull merged commit 6c5d4a2 into lightningdevkit:main Mar 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants