Opened 7 years ago

Closed 7 years ago

#9644 closed defect (fixed)

Memory leak in get_cert_list() when the client bootstraps

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


Running release-0.2.4 (git commit 20d4356c3) as a Tor client. I let it bootstrap its directory info, and then kill it.

==27498== 216 bytes in 9 blocks are definitely lost in loss record 14 of 21
==27498==    at 0x4C28BED: malloc (in /usr/lib/valgrind/
==27498==    by 0x222FE7: tor_malloc_ (util.c:143)
==27498==    by 0x21AC5D: digestmap_new (container.c:1038)
==27498==    by 0x15A63E: get_cert_list (routerlist.c:48)
==27498==    by 0x15B7FD: authority_certs_fetch_missing (routerlist.c:692)
==27498==    by 0x127848: networkstatus_set_current_consensus (networkstatus.c:1724)
==27498==    by 0x1E652B: connection_dir_client_reached_eof (directory.c:1900)
==27498==    by 0x1E7F68: connection_dir_reached_eof (directory.c:2356)
==27498==    by 0x1C2406: connection_handle_read (connection.c:4122)
==27498==    by 0x11DFE0: conn_read_callback (main.c:718)
==27498==    by 0x52D8CCB: event_base_loop (in /usr/lib/x86_64-linux-gnu/
==27498==    by 0x11E974: do_main_loop (main.c:1987)

Child Tickets

Change History (6)

comment:1 Changed 7 years ago by arma

Keywords: tor-client added

comment:2 Changed 7 years ago by nickm

Keywords: small-leak added

This would appear to be a singleton leak of trusted_dir_certs in routerlist.c; the solution is for it and its contents to be freed by routerlist_free_all. But that appears to already happen in routerlist_free_all. Very strange.

comment:3 Changed 7 years ago by nickm

Priority: normalminor
Status: newneeds_review

Aha. We weren't freeing the dl_status_map field.

Please review branch "bug9644_024" in my public repository.

comment:4 Changed 7 years ago by sysrqb

I reproduced arma's output, bug9644_024 looks good to me and plugs the leak.

comment:5 Changed 7 years ago by nickm

Okay, merging. Thanks for the review!

comment:6 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed
Note: See TracTickets for help on using tickets.