Opened 7 years ago

Closed 5 years ago

#7471 closed defect (fixed)

circuit_unlink_all_from_channel() is brain-damaged

Reported by: andrea Owned by: andrea
Priority: Medium Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version: Tor: 0.2.4.5-alpha
Severity: Keywords: tor-relay, 025-triaged
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The circuit_unlink_all_from_channel() function calls channel_unlink_all_circuits() and then circuit_mark_for_close() on each circuit in a loop. The channel_unlink_all_circuits() call resets the channel's num_n_circuits and num_p_circutis to 0, and then they get decremented, which causes them to wrap back below 0, and in the case of spliced rendezvous circuits the circuit_mark_for_close() after detachment from the cmux in channel_unlink_all-circuits() can lead to a spurious circuit_clear_cell_queue() with no cmux to update on. This function should be rewritten to be less stupid.

Child Tickets

Change History (7)

comment:1 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:2 Changed 6 years ago by nickm

The description makes this sound worth cleaning, but not actually a bug. Is that so? If so, shall we postpone it to 0.2.5?

comment:3 Changed 6 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: 0.2.5.x-final

comment:4 Changed 5 years ago by nickm

See also #9683, where I rewrote it not to be a performance bottleneck. Maybe that ticket adds sanity too.

comment:5 Changed 5 years ago by andrea

Triage for 0.2.5.x: it sounds likely that #9683 will be mergeable pending testing of the proposed circuit_mark_for_close_() change, so this and #9683 should remain in 0.2.5.x.

comment:6 Changed 5 years ago by nickm

Keywords: 025-triaged added

comment:7 Changed 5 years ago by nickm

Resolution: fixed
Status: newclosed

#9683 is merged; closing this.

Note: See TracTickets for help on using tickets.