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: fix abba bug in UsefullNewPeer #122

Merged
merged 3 commits into from
Jun 15, 2023
Merged

fix: fix abba bug in UsefullNewPeer #122

merged 3 commits into from
Jun 15, 2023

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Jun 15, 2023

This fixes a deadlock the issue is that this pattern could happen because .Find inside UsefullNewPeer would aquire the same RLock:

  • UsefullNewPeer takes tablock.RLock
  • Something else tries to take tablock.Lock (waits)
  • UsefullNewPeer calls Find calls NearestPeer call NearestPeers which trie to take tablock.RLock again, here RLock waits because the rwmutex is starving, it will wait for the Something else to aqcuire the Lock first else a constant flow of reader would block the RWMutex forever.

Jorropo added 2 commits June 15, 2023 02:38
This fixes a deadlock the issue is that this pattern could happen because .Find inside UsefullNewPeer would aquire the same RLock:
- UsefullNewPeer takes tablock.RLock
- Something else tries to take tablock.Lock (waits)
- UsefullNewPeer calls Find calls NearestPeer call NearestPeers which trie to take tablock.RLock again, here RLock waits because the rwmutex is starving, it will wait for the Something else to aqcuire the Lock first else a constant flow of reader would block the RWMutex forever.
@Jorropo Jorropo requested a review from guillaumemichel June 15, 2023 00:42
@Jorropo Jorropo marked this pull request as draft June 15, 2023 00:54
@Jorropo Jorropo marked this pull request as ready for review June 15, 2023 00:59
@BigLep BigLep mentioned this pull request Jun 15, 2023
@github-actions
Copy link

Suggested version: v0.6.3

Comparing to: v0.6.2 (diff)

Changes in go.mod file(s):

(empty)

gorelease says:

# github.com/libp2p/go-libp2p-kbucket
## incompatible changes
(*RoutingTable).GenRandomKey: removed
(*RoutingTable).UsefulNewPeer: removed

# diagnostics
go.mod: the following requirements are needed
	github.com/ipfs/[email protected]
Run 'go mod tidy' to add missing requirements.

# summary
Suggested version: v0.7.0

gocompat says:

Your branch is up to date with 'origin/master'.

Cutting a Release (and modifying non-markdown files)

This PR is modifying both version.json and non-markdown files.
The Release Checker is not able to analyse files that are not checked in to master. This might cause the above analysis to be inaccurate.
Please consider performing all the code changes in a separate PR before cutting the release.

Automatically created GitHub Release

A draft GitHub Release has been created.
It is going to be published when this PR is merged.
You can modify its' body to include any release notes you wish to include with the release.

Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

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

Thanks a lot for spotting the bug @Jorropo

@guillaumemichel guillaumemichel merged commit 437b9fc into master Jun 15, 2023
@guillaumemichel guillaumemichel deleted the fix-abba-bug branch June 15, 2023 06:01
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