Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#9108 closed enhancement (implemented)

Make circuitbuild.c: global_circuitlist a proper doubly-linked list

Reported by: marek Owned by:
Priority: Low Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I had a go at #4519, and instead of doing the thing described there I decided to simplify the code and make the big global_circuitlist list a nice doubly-linked list.

Child Tickets

Attachments (2)

Change History (9)

comment:1 Changed 6 years ago by marek

Status: newneeds_review

comment:2 Changed 6 years ago by marek

Argh. There's a typo in the second patch. Better use branch bug9108 on https://github.com/majek/tor.git .

comment:3 Changed 6 years ago by nickm

I think this is a good idea. I'm was confused by moving the remove code into circuit_free, but I think it does make sense there. We should audit every caller to circuit_free, of course. I was also worried about not using FOREACH_SAFE for the loop that has a local variable called "victim", but apparently that "victim" is only at risk of being marked, not freed. I should double-check this all more carefully rsn.

comment:4 in reply to:  3 Changed 6 years ago by marek

Replying to nickm:

I think this is a good idea. I'm was confused by moving the remove code into circuit_free, but I think it does make sense there. We should audit every caller to circuit_free, of course.

There are two of those. 429:circuit_close_all_marked and 810:circuit_free_all and wrapped in FOREACH_SAFE. I'm fairly sure there isn't a memory leak there.

comment:5 Changed 6 years ago by nickm

Keywords: tor-client added
Milestone: Tor: unspecifiedTor: 0.2.5.x-final

comment:6 Changed 6 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Okay; still looks fine to me. Merged. Thanks!

comment:7 Changed 6 years ago by arma

Nickm: fyi, your ca6949ebe commit (adding a ticket9108 file) was redundant since it already had a bug9108 file.

That said, I will use (mostly) your changes text, so is fine. :)

Note: See TracTickets for help on using tickets.