Opened 12 months ago

Closed 3 weeks ago

#20059 closed defect (fixed)

Bug: Duplicate call to circuit_mark_for_close

Reported by: cypherpunks Owned by: nickm
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 030-backport, 029-backport
Cc: tor@… Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

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.

Child Tickets

Change History (14)

comment:1 Changed 12 months ago by cypherpunks

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

comment:2 Changed 12 months ago by nickm

Milestone: Tor: 0.2.8.x-final

comment:3 Changed 9 months ago by nickm

Status: newneeds_information

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

comment:5 Changed 7 months ago by s7r

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 was more or less kind of related, but somehow the fix did not cover this part as well.

comment:6 Changed 6 months ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.3.1.x-final

comment:7 Changed 5 months ago by nickm

Owner: set to nickm
Status: needs_informationaccepted

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);

comment:8 Changed 5 months ago by nickm

Keywords: 030-backport 029-backport added
Status: acceptedneeds_review

Likely fix in bug20059_024.

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

comment:9 Changed 5 months ago by nickm

Keywords: review-group-17 added

comment:10 Changed 5 months ago by dgoulet

Reviewer: dgoulet
Status: needs_reviewmerge_ready

lgtm and ack for the backport.

comment:11 Changed 5 months ago by nickm

Keywords: review-group-17 removed
Milestone: Tor: 0.3.1.x-finalTor: 0.2.9.x-final
Status: merge_readyneeds_review

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

comment:12 Changed 5 months ago by Christian

Cc: tor@… added

comment:13 Changed 3 months ago by nickm

Status: needs_reviewmerge_ready

comment:14 Changed 3 weeks ago by nickm

Resolution: fixed
Status: merge_readyclosed

merged to 0.2.9 as trivially correct.

Note: See TracTickets for help on using tickets.