[tor-bugs] #11069 [Tor]: Tor with unreachable PTs should not hang the bootstrap process

[Tor Bug Tracker & Wiki]

#11069: Tor with unreachable PTs should not hang the bootstrap process
------------------------+------------------------------------------
     Reporter:  asn     |      Owner:
         Type:  defect  |     Status:  needs_review
     Priority:  normal  |  Milestone:  Tor: 0.2.5.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  tor-pt tor-client, tbb-needs
Actual Points:          |  Parent ID:
       Points:          |
------------------------+------------------------------------------

Comment (by andrea):

 Replying to [comment:11 asn]:
 > OK. See branch `bug11069` in
 `https://git.torproject.org/user/asn/tor.git`.
 >
 > After some testing it seems to work fine.
 >
 > A thing I'm not so sure about is that I moved
 `connection_or_notify_error()` after the connection has been marked for
 closing. There doesn't seem to be anything wrong with that, but that
 function also calls `channel_close_for_error()` and I'm not sure if the
 channel subsystem cares about whether the connection has been closed or
 not.
 >
 > Also, are there any other attributes of the connection that we should be
 checking in `any_active_or_conns()`? We could check whether its state is
 `OR_CONN_STATE_CONNECTING` for example, but I'm not sure if this is a good
 idea.

 Bad things will happen if the connection gets closed without channels
 being notified properly because then there will be a channel_t hanging
 around with a bad pointer to an or_connection_t that doesn't exist any
 more.  You have to ensure that you're calling channel_close_for_error() or
 one of the other close-from-below functions when you close an
 or_connection_t.

 It looks like you're doing that and changing the order, with
 connection_mark_for_close_internal() before channel_close_for_error() - I
 *believe* this is safe *as long as you're certain* that
 channel_close_for_error() happens before close_closeable_connections()
 gets to that orconn.  Is it really necessary to structure it like that
 though, different from the order of all the other calls to
 channel_close_for_error() in connection.c?  If so, perhaps an explanatory
 comment would be advisable.

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