[tor-bugs] #1184 [Tor Client]: Sending useless relay cells after the destroy cell

[Tor Bug Tracker & Wiki]

#1184: Sending useless relay cells after the destroy cell
--------------------------------+-------------------------------------------
 Reporter:  arma                |         Type:  defect    
   Status:  needs_review        |     Priority:  minor     
Milestone:  Tor: 0.2.2.x-final  |    Component:  Tor Client
  Version:  0.2.2.6-alpha       |   Resolution:  None      
 Keywords:                      |       Parent:            
--------------------------------+-------------------------------------------

Comment(by nickm):

 Let me explain in detail.

 I rather suspect that this check ''is'' necessary:
 append_cell_to_circuit_queue() is called by:

   * circuit_receive_relay_cell, which checks for marked_for_close.
   * circuit_package_relay_cell, which does ''not'' check, and is called
 by:
     * relay_send_command_from_edge, which does not check, and is called
 by:
       * '''Roughly 20 different places throughout the code'''
   * circuit_deliver_create_cell, which does not check, and is called by:
     * circuit_n_conn_done, which does check.
     * circuit_extend, which does not check, and is called by:
       * connection_edge_process_relay_cell, which is called by:
          * circuit_receive_relay_cell, which checks.
   * onionskin_answer, which does not check, and is called by:
     * command_process_create_cell(), which does not need to check since it
 creates the circuit in question.
     * connection_cpu_process_inbuf(), which checks implicitly by
 retrieving the circuit using circuit_get_by_circid_orconn().

 So modulo the 20 callers of relay_send_command_from_edge, then yeah, I am
 reasonably certain that we don't need to add the check.  But becoming
 reasonably certain about this is hard and time-consuming, and that itself
 suggests a problem in this part of our code: if I want to make sure that
 some X never happens if Y is true, I shouldn't have to trace all parts of
 the call graph that lead to X to make sure that Y is checked along each
 one.  Putting a check at the position right before X is done is a
 '''better''' way to ensure that X never happens if Y than instituting a
 code-wide rule of "When you call the thing that does X, make sure you have
 checked Y, or every path that leads to you has checked Y."

 But with the change in 87f18c95783, I'm never going to have to waste my
 time doing this analysis again. And isn't that nice?

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