[tor-bugs] #18809 [Core Tor/Tor]: Handle linked connections better during bootstrap

Tor Bug Tracker & Wiki blackhole at torproject.org
Mon May 9 17:00:50 UTC 2016


#18809: Handle linked connections better during bootstrap
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  andrea
     Type:  defect                               |         Status:
 Priority:  Medium                               |  needs_review
Component:  Core Tor/Tor                         |      Milestone:  Tor:
 Severity:  Normal                               |  0.2.8.x-final
 Keywords:  must-fix-before-028-rc,              |        Version:  Tor:
  TorCoreTeam201605                              |  0.2.8.1-alpha
Parent ID:                                       |     Resolution:
 Reviewer:  teor                                 |  Actual Points:
                                                 |         Points:
                                                 |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by andrea):

 {{{
 Begin code review for arma's bug18809 branch:

 525307c0ea5717adba9f35cf7ccb1c5108ac3440:
  - Looks fine, just typo fixes

 33335b803c346a49513e4c64848342eedb6de77c:
  - I think this is correct.
  - Maybe break out of the loop in
 networkstatus_consensus_is_already_downloading()
    for efficiency after answer = 1?

 5c81825104f063c69ba17a5b2545f43756d534d6:
  - Looks good to me.

 a9012a61fafae3ca8c6ae40ba5f433dcf12aad71:
  - IIUC, we're replacing a complicated algorithm for picking which active
    connection to keep meant to run periodically with one that takes which
    one to keep passed in as an argument and triggered from various places
    that can cause us to want to kill existing connections - like
 successfully
    downloading the consensus, leading to all this simplification and
 deleted
    code.
  - I think this is all okay.

 7df6a08ff84a3925ce4fe3c49b86d2033c4e3d58:
  - We're removing a lot of places a connection could get closed; objective
    is to make sure they hang around until something succeeds in
 downloading
    the consensus, as per the invocation of
 connection_dir_close_consensus_fetches()
    in a9012a61fafae3ca8c6ae40ba5f433dcf12aad71, I presume?
  - I suppose if everything just hangs and never succeeds, they eventually
 go
    away due to network timeouts anyway.
  - I think this is okay.

 6642bc346edb30f068c554fa674062930ad315c0:
  - This looks fine.

 015472016cb6c328a80b02da14f16be05f208ebb:
  - Looks good to me.

 26b64a79bed253bf08b3d65fa360419af2c0065c:
  - This looks okay.

 005617c92142dcaa7c76152d3e1c007585bf81cc:
  - This looks okay.

 0a7c177336dab1fdb04e46e33230191bb4834793:
  - Okay, this covered my suggestion in
 33335b803c346a49513e4c64848342eedb6de77c.

 I think this is fine to merge provided it's been tested in Chutney or
 equivalent
 that covers all the roles a Tor process can bootstrap in.

 End code review
 }}}

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18809#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list