[tor-bugs] #2149 [Tor Client]: new 'extra dormant' mode for people who never use their tor

[Tor Bug Tracker & Wiki]

#2149: new 'extra dormant' mode for people who never use their tor
---------------------------------+------------------------------------------
 Reporter:  arma                 |          Owner:                    
     Type:  enhancement          |         Status:  needs_revision    
 Priority:  major                |      Milestone:  Tor: 0.2.4.x-final
Component:  Tor Client           |        Version:                    
 Keywords:  performance scaling  |         Parent:                    
   Points:                       |   Actualpoints:                    
---------------------------------+------------------------------------------
Changes (by nickm):

  * status:  needs_review => needs_revision


Comment:

 Thanks for the code!  Just pulled the branch -- The "Using wrong idle
 checker" patch doesn't seem to be there yet?

 Quick review:
    * IDLE_TIME_STAGE isn't used.
    * stopped_fetching_descriptors isn't documented.
    * needs a changelog entry in the changes directory.
    * or_options_t is for stuff parsed from torrc and the command line --
 it isn't a catch-all for persistent state.  That would be or_state_t.
 Making this change will also let you back out lots of the
 get_options_mutable() calls, which ought to be a red flag.
    * I don't see the point of having DormantStage be global: why can't it
 be a local variable in directory_too_idle_to_fetch_descriptors() ?
    * I don't understand the logic of the changes in conn_read_callback and
 do_main_loop.  In any case, it feels like "directory_info_has_arrived()"
 might be the wrong function to be calling here -- after all, you are
 calling it *not* in response to directory info arriving.[*]
    * The comment on 59c8df8b59bf04f25 seems wrong; conn_read_callback
 isn't the function that gets called on "an incoming connection" -- it gets
 called every time any socket is readable.  If you want to have something
 happen on incoming connections, that would belong in
 connection_handle_listener_read, I think.

 [*] Functions should be called as documented, and not because they have
 some desirable side-effect.  Let's say there is a function called
 computer_is_on_fire() that floods the server room with carbon dioxide. It
 is IMO wrong to call computer_is_on_fire() when the computer is not on
 fire.  If we sometimes want to flood the server room with CO2 when there
 is no fire, then we should make an flood_server_room() function.  That
 makes our code more maintainable and less confusing.

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