Move most of "circuit_mark_for_close" into "circuit_free" or "circuit_close_all_marked".

The function "circuit_mark_for_close()" calls a few other functions (notably circuit_build_failed() and rend_client_report_intro_point_failure()) that call into the blob. If we pull it out, we would chop the remaining blob in half.

We should use a system similar to the one we use for closeable connections where we put closeable circuits on a list, rather than iterating over the entire circuit list.

Doing this would remove more than half of the functions currently in the blob as currently measured. (!)

changed milestone to %Tor: 0.2.8.x-final

added component::core tor/tor milestone::Tor: 0.2.8.x-final parent::16764 priority::medium resolution::implemented severity::major sponsor::S status::closed type::enhancement labels

my branch decouple_circuit_mark tries to do this.

Trac:
Status: new to needs_review

This is currently running on my fast relay, we'll see in the coming days if any issues arise.

With master at commit 19e10f95c105a698d3593a1ecfa88a7449a21127, when sending SIGTERM to tor, here is a backtrace of an assertion:

Oct 27 14:23:33.000 [notice] Catching signal TERM, exiting cleanly.
Oct 27 14:23:33.000 [warn] Couldn't find circuit 4107801626 (for channel 1232)
Oct 27 14:23:33.000 [err] tor_assertion_failed_(): Bug: src/or/circuitmux.c:501: circuitmux_detach_all_circuits: Assertion to_remove->muxinfo.policy_data == NULL failed; aborting. (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug: Assertion to_remove->muxinfo.policy_data == NULL failed in circuitmux_detach_all_circuits at src/or/circuitmux.c:501. Stack trace: (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(log_backtrace+0x41) [0x7f4d6c206981] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(tor_assertion_failed_+0x94) [0x7f4d6c2145b4] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(circuitmux_detach_all_circuits+0x2a1) [0x7f4d6c195c91] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(+0xa5cae) [0x7f4d6c182cae] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(channel_free_all+0x8f) [0x7f4d6c18319f] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(tor_free_all+0x7a) [0x7f4d6c11937a] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(tor_cleanup+0x2c) [0x7f4d6c1194ec] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(+0x3d664) [0x7f4d6c11a664] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     /usr/lib/x86_64-linux-gnu/libevent-2.0.so.5(event_base_loop+0xa7b) [0x7f4d6b76324b] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(do_main_loop+0x20d) [0x7f4d6c11884d] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(tor_main+0x19ad) [0x7f4d6c11c96d] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(main+0x19) [0x7f4d6c115ba9] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7f4d6ab59ec5] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)
Oct 27 14:23:33.000 [err] Bug:     ./git/tor/src/or/tor(+0x38bfb) [0x7f4d6c115bfb] (on Tor 0.2.8.0-alpha-dev 81b6f18dbef9187b)

Trac:
Severity: N/A to Normal

I just got this (couple minutes after start):

Oct 27 14:26:26.Oct 27 14:26:26.000 [warn] Remote server sent bogus reason code 65023

dgoulet has still been testing this, and hasn't seen anything weird. That's pretty neat!

So at this point, I'm fairly confident this won't kill relays. I wonder if it will be bad for clients or HS services.

Tried it for a while, and it seemed to work for me. It passed all the tests, including test-stem and test-network-all. It's merging time!

Trac:
Status: needs_review to closed
Resolution: N/A to implemented

The blob is now down to 38 functions!!

comment:3 is still happening every time tor stops.

Trac:
Severity: Normal to Major
Resolution: implemented to N/A
Status: closed to reopened

(Could we have a new ticket for that one? This ticket actually worked.)

Trac:
0001-Fix-memory-leak-by-circuit-marked-for-close-list.patch

The commit implementing this ticket (8b4e5b7ee902fb7fa07767410a18433d752c7aef) introduces a memory leak by not freeing the memory held by the new circuit list when tor tries to free its resources. The attached patch fixes the issue.

Trac:
Status: reopened to needs_review

This is a simple patch to fix a memory leak, let's get it merged.

Merged!

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

closed

mentioned in issue #20423 (moved)

moved to tpo/core/tor#17218 (closed)