[tor-bugs] #3443 [Tor]: Client with low CBT can't establish any circuits

[Tor Bug Tracker & Wiki]

#3443: Client with low CBT can't establish any circuits
---------------------------------------------------+------------------------
 Reporter:  arma                                   |          Owner:                    
     Type:  defect                                 |         Status:  needs_review      
 Priority:  major                                  |      Milestone:  Tor: 0.2.4.x-final
Component:  Tor                                    |        Version:                    
 Keywords:  regression tor-client MikePerry201211  |         Parent:                    
   Points:                                         |   Actualpoints:  6                 
---------------------------------------------------+------------------------

Comment(by mikeperry):

 Replying to [comment:45 nickm]:
 > Dumb question:
 >
 > * I can't see what actually relaxes the timeout when relaxed_timeout is
 set.  It appears that the first time through the circuit_expire_building
 loop, we set relaxed_timeout , but the next time through the loop, we'll
 see that relaxed_timeout is already set, and call the "No circuits are
 opened" log message", and go on.  What's supposed to be happening here?  I
 thought the logic was that a relaxed_timeout circuit should get a more lax
 timeout, but it seems like everybody's getting the same close_cutoff.

 The logic is "If there are no opened circuits, allow *every* circuit to
 wait until the close_cutoff, but mark each one that we decide to do this
 for, so we can both avoid using them later unless we have to, and also do
 some CBT bookkeeping. As soon as there are *any* opened circuits, we can
 stop doing this relaxed timeout stuff and even let all the unbuilt relaxed
 circuits we were waiting on expire immediately."

 However, I've decided to change the "any_opened_circuits" check to be "Are
 there any opened 3 hop circuits that we're not trying to cannibalize?"
 Otherwise, I think we're going to count open 1-hop and other weird
 circuits to mean we shouldn't relax, which is not quite right.

 I've also upgraded the log message where a relaxed circuit *still* isn't
 opened by the close_cutoff to a notice. If we get all the way to that
 point with no circuits opened, it might mean more trouble/bugs.

 > Should get fixed, not necessarily before merge:
 >
 > * The new while loop in circuit_expire_building uses a silly form.  It's
 not necessary to have all this "victim" business when you're not modifying
 the list of circuits-- the only reason to use that pattern is if you think
 you might be removing "victim" from the list.
 > * In the later while loop in circuit_expire_building, you're checking
 whether TO_ORIGIN_CIRCUIT(victim)->cpath is set, but earlier in the
 function, you're doing "if (TO_ORIGIN_CIRCUIT(victim)->cpath->state ==
 CPATH_STATE_CLOSED) {"
 > * Describe_status_for_controller probably wants to take the created
 time, so as not to change its behavior.
 > * The documentation for timestamp_began and timestamp_created should
 explain how they're different.

 All of these should be fixed in the latest commit to mikeperry/bug3443.

 The doc update also made me realize we should kill the timestamp_created
 update during cannibalize. We should only update timestamp_began there.

 > Other than that, if it's testing ok, I say it's mergeable.

 Should I squash it down? Should this all be just one commit?

 As a heads up, I also plan to rebase my other two branches (#7157 and
 #7341) to be on top of this one, to make testing all of them easier (since
 all three can be exercised with the same test).

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