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

Jump to bottom

[Tor Trac #31652]: hs-v3: Service circuit retry limit should not close a valid circuit #1297

Closed
wants to merge 1 commit into from
Closed

[Tor Trac #31652]: hs-v3: Service circuit retry limit should not close a valid circuit #1297

wants to merge 1 commit into from

Conversation

Reviewers

@dgoulet-tor dgoulet-tor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@neelchauhan @coveralls @dgoulet-tor @torproject-pusher
@neelchauhan
Copy link
Contributor

@neelchauhan neelchauhan commented Sep 5, 2019

For the bug here.
@coveralls
Copy link

@coveralls coveralls commented Sep 5, 2019 •
edited

Pull Request Test Coverage Report for Build 6088

  • 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.0008%) to 63.073%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/hs/hs_service.c 0 2 0.0%
Totals Coverage Status
Change from base Build 6080: -0.0008%
Covered Lines: 47621
Relevant Lines: 75502
💛 - Coveralls

@neelchauhan neelchauhan force-pushed the bug31652 branch 2 times, most recently from d0767d4 to 6ca485f Sep 5, 2019
@neelchauhan
Don't retry v3 intro circuits more than three times
6ca485f
dgoulet-tor
dgoulet-tor reviewed Sep 10, 2019
View changes
src/feature/hs/hs_service.c
* In this case, we remove the intro point and a new one will be picked
* at the next main loop callback. */
if (ei == NULL ||
ip->circuit_retries > MAX_INTRO_POINT_CIRCUIT_RETRIES) {
Copy link
Contributor

@dgoulet-tor dgoulet-tor Sep 10, 2019

So unfortunately, we can't do this here. Problem is that this IP needs to be remembered in our "failing cache", see remember_failing_intro_point().

I think the IP should be cleaned up before we get into the build circuit main loop event which calls this function.

The obvious candidate is cleanup_intro_points() which is where it is currently happening but I think the checks needs to be something around:

"if too many retries and no existing circuits: cleanup()"

@torproject-pusher torproject-pusher deleted the branch torproject:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment