Bug: Duplicate call to circuit_mark_for_close

I noticed an extreme increase of memory usage (increase by ~500MB compared to normal), after looking into the logs I stumbled upon:

circuit_mark_for_close_(): Bug: Duplicate call to circuit_mark_for_close at ../src/or/onion.c:182 (first at ../src/or/command.c:559) (on Tor 0.2.8.6 )

(6 times within one second)

also in the logs 10 seconds before the log entries above:

Removed 109202016 bytes by killing 2 circuits; 5252 circuits remain alive. Also killed 0 non-linked directory connections.

changed milestone to %Tor: 0.2.9.x-final

added 029-backport 030-backport component::core tor/tor milestone::Tor: 0.2.9.x-final owner::nickm priority::medium resolution::fixed reviewer::dgoulet severity::normal status::closed type::defect labels

Sounds a bit like #7591 (moved) but with a different function. The 6 times within one second thing is the same though.

Trac:
Milestone: N/A to Tor: 0.2.8.x-final

This also reminds me of #20203 (moved), which we fixed. Is this still happening?

Trac:
Status: new to needs_information

Another relay operator observes this issue: https://lists.torproject.org/pipermail/tor-relays/2017-January/011770.html

This was just reported today again on tor-relays@

Feb 05 15:01:25.000 [warn] assign_to_cpuworker failed. Ignoring.
Feb 05 15:01:29.000 [warn] circuit_mark_for_close_: Bug: Duplicate call
to circuit_mark_for_close at src/or/onion.c:238 (first at
src/or/command.c:579) (on Tor 0.2.9.8 01ab67e38b358ae9)
Feb 05 15:01:36.000 [warn] assign_to_cpuworker failed. Ignoring.

Apparently #20423 (moved) was more or less kind of related, but somehow the fix did not cover this part as well.

Trac:
Milestone: Tor: 0.2.8.x-final to Tor: 0.3.1.x-final

Okay; the first close is here in command.c:

    /* the destroy came from behind */
    circuit_set_p_circid_chan(TO_OR_CIRCUIT(circ), 0, NULL);
    circuit_mark_for_close(circ, reason|END_CIRC_REASON_FLAG_REMOTE);

And the second is here in onion.c:

    log_info(LD_CIRC,
             "Circuit create request is too old; canceling due to overload.");
    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT);

Trac:
Status: needs_information to accepted
Owner: N/A to nickm

Likely fix in bug20059_024.

I say we only take this in 029 and forward, since it's just a warning.

Trac:
Keywords: N/A deleted, 030-backport 029-backport added
Status: accepted to needs_review

Trac:
Keywords: 030-backport 029-backport deleted, 030-backport, 029-backport, review-group-17 added

lgtm and ack for the backport.

Trac:
Reviewer: N/A to dgoulet
Status: needs_review to merge_ready

Whoops. It didn't actually compile. Fixed in a fixup commit, squashed in bug20059_024_v2.

Taken in 030 and forward, marking for possible 029 backport

Trac:
Status: merge_ready to needs_review
Milestone: Tor: 0.3.1.x-final to Tor: 0.2.9.x-final
Keywords: review-group-17 deleted, N/A added

Trac:
Username: Christian
Cc: N/A to tor@nerdbynature.de

Trac:
Status: needs_review to merge_ready

merged to 0.2.9 as trivially correct.

Trac:
Status: merge_ready to closed
Resolution: N/A to fixed

closed

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