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 CachePeer.cc:101 "!tcp_up" assertion after cache_peer death #1962

Conversation

eduard-bagdasaryan
Copy link
Contributor

getFirstUpParent() could return a down (or otherwise disqualified)
cache_peer. Broken by 2023 commit 2e24d0b.

Refactored function structure to improve code quality, partially
addressing "use our return/reporting style" TODO in getDefaultParent().

eduard-bagdasaryan and others added 2 commits December 12, 2024 14:10
getFirstUpParent() could return a non-nil cache_peer
even if the search failed. Broken since commit 2e24d0b.
Also removed an unwanted empty line.
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this bug and checking other CurrentCachePeers() loops for similar problems.

@rousskov rousskov added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels S-could-use-an-approval An approval may speed this PR merger (but is not required) labels Dec 13, 2024
Copy link
Contributor

@kinkie kinkie left a comment

Choose a reason for hiding this comment

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

thank you!

squid-anubis pushed a commit that referenced this pull request Dec 15, 2024
getFirstUpParent() could return a down (or otherwise disqualified)
cache_peer. Broken by 2023 commit 2e24d0b.

Refactored function structure to improve code quality, partially
addressing "use our return/reporting style" TODO in getDefaultParent().
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Dec 15, 2024
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Dec 15, 2024
@rousskov rousskov removed the S-could-use-an-approval An approval may speed this PR merger (but is not required) label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants