Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T14:58:32Zhttps://gitlab.torproject.org/legacy/trac/-/issues/19328Try not to log from inside functions called from inside log functions2020-06-13T14:58:32ZNick MathewsonTry not to log from inside functions called from inside log functionsOur logging code is technically recursive now. Logging asks what the time is and tries to format it. Formatting the time can fail. Failures can log.Our logging code is technically recursive now. Logging asks what the time is and tries to format it. Formatting the time can fail. Failures can log.Tor: 0.3.5.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/17590Decouple connection_ap_handshake_attach_circuit from nearly everything.2020-06-13T14:52:43ZNick MathewsonDecouple connection_ap_handshake_attach_circuit from nearly everything.Long ago we used to call connection_ap_handshake_attach_circuit() only in a few places, since connection_ap_attach_pending() attaches all the pending connections, and does so regularly. But this turned out to have a performance problem:...Long ago we used to call connection_ap_handshake_attach_circuit() only in a few places, since connection_ap_attach_pending() attaches all the pending connections, and does so regularly. But this turned out to have a performance problem: it would introduce a delay to launching or connecting a stream.
We couldn't just call connection_ap_attach_pending() every time we make a new connection, since it walks the whole connection list. So we started calling connection_ap_attach_pending all over, instead! But that's kind of ugly and messes up our callgraph.
But we have an opportunity to make Tor simpler!
* We can make connection_ap_attach_pending() linear in the number of pending entry connections, rather than in the number of total connections.
* If we do that, we can make it get called from whenever there is a pending entry connection from run_main_loop_once() or somewhere.
* And if we do that, we can just put connections on a pending-list, rather than calling connection_ap_attach_pending() on them directly.
This will simplify tor, simplify our callgraph, and -- with the help of #17589 -- break the blob into multiple smaller strongly connected components.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/17589Decouple connection_dir_request_failed() from directory_initiate_command_rend()2020-06-13T14:50:55ZNick MathewsonDecouple connection_dir_request_failed() from directory_initiate_command_rend()Instead of calling connection_dir_request_failed() directly, directory_initiate_command_rend() should mark the failed connection and have it cleaned up later. This would prevent recursive invocation of directory_initiate_command_rend(), ...Instead of calling connection_dir_request_failed() directly, directory_initiate_command_rend() should mark the failed connection and have it cleaned up later. This would prevent recursive invocation of directory_initiate_command_rend(), and remove 12 functions from the Blob.
I'd do this right now, but I want to test that the code actually does the right thing.Tor: 0.2.8.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/16789Don't launch downloads directly from routerlist_retry_directory_downloads()2020-06-13T14:47:59ZNick MathewsonDon't launch downloads directly from routerlist_retry_directory_downloads()It would be smarter to just clear the timeouts and clear the timers that control this, and allow the scheduler to launch them itself. That would pull another 9 functions out of the blob.It would be smarter to just clear the timeouts and clear the timers that control this, and allow the scheduler to launch them itself. That would pull another 9 functions out of the blob.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/16788Don't call tor_cleanup() directly from lost_owning_controller()2020-06-13T14:47:59ZNick MathewsonDon't call tor_cleanup() directly from lost_owning_controller()lost_owning_controller should just schedule our SIGTERM handler to get run. This will remove a tiny bit of duplicated code and simplify our callgraphlost_owning_controller should just schedule our SIGTERM handler to get run. This will remove a tiny bit of duplicated code and simplify our callgraphTor: 0.2.7.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/16764Simplify Tor's control flow graph to the extent we can.2020-06-13T15:32:14ZNick MathewsonSimplify Tor's control flow graph to the extent we can.For background, see http://archives.seul.org/tor/dev/Mar-2015/msg00197.html .
As of this writing, the largest strongly-connected component in Tor's callgraph has 407 functions in it. Nobody can actually understand a program that's so c...For background, see http://archives.seul.org/tor/dev/Mar-2015/msg00197.html .
As of this writing, the largest strongly-connected component in Tor's callgraph has 407 functions in it. Nobody can actually understand a program that's so complex. Let's simplify it!
(This is a parent ticket.)Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/16763Extract client-only parts of init_keys()2020-06-13T14:47:53ZNick MathewsonExtract client-only parts of init_keys()init_keys() is one of the functions that's reachable from way too many functions. This appears to be happening because ip_address_changed can call init_keys(). But really, it only wants to call the client-only parts of init_keys(). So...init_keys() is one of the functions that's reachable from way too many functions. This appears to be happening because ip_address_changed can call init_keys(). But really, it only wants to call the client-only parts of init_keys(). So splitting them out into another function would help reduce the blob size.
See also #16695 and #16762.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/16762Move most of directory_all_unreachable into a backend callback2020-06-13T14:47:53ZNick MathewsonMove most of directory_all_unreachable into a backend callbackAfter my patch on #16695, the blob becomes smaller, but still quite scary (311 functions in our biggest SCC). If we move the body of directory_all_unreachable into a callback, we can chop that by nearly half.After my patch on #16695, the blob becomes smaller, but still quite scary (311 functions in our biggest SCC). If we move the body of directory_all_unreachable into a callback, we can chop that by nearly half.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/legacy/trac/-/issues/16695Decouple generating controller events from sending them to controllers2020-06-13T14:47:43ZNick MathewsonDecouple generating controller events from sending them to controllersIn [my analysis of Tor's callgraph](http://archives.seul.org/tor/dev/Mar-2015/msg00197.html) , I found that a lot of our callgraph's complexity comes from the fact that all the code that generates a controller event can try to call into ...In [my analysis of Tor's callgraph](http://archives.seul.org/tor/dev/Mar-2015/msg00197.html) , I found that a lot of our callgraph's complexity comes from the fact that all the code that generates a controller event can try to call into the network stack. And since controller events can come from nearly anywhere in the code, that's quite a problem for modularity.
So, let's try to make the "blob" smaller by decoupling the logic here.Tor: 0.2.7.x-final