Opened 6 years ago

Closed 6 years ago

#7029 closed defect (fixed)

some stuff doesn't get freed at exit

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

Description

==31899== 20 bytes in 1 blocks are still reachable in loss record 3 of 25
==31899==    at 0x4C28BED: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31899==    by 0x1FF567: _tor_malloc (util.c:144)
==31899==    by 0x1FF665: _tor_malloc_zero (util.c:170)
==31899==    by 0x1797C4: circuit_build_times_init (circuitbuild.c:597)
==31899==    by 0x17B89F: circuit_build_times_parse_state (circuitbuild.c:955)
==31899==    by 0x172231: or_state_load (statefile.c:238)
==31899==    by 0x19AFCD: set_options (config.c:1254)
==31899==    by 0x19BDB8: options_init_from_string (config.c:3639)
==31899==    by 0x19C11E: options_init_from_torrc (config.c:3496)
==31899==    by 0x11E097: tor_init (main.c:2347)
==31899==    by 0x11F2CD: tor_main (main.c:2665)
==31899==    by 0x5F7BEAC: (below main) (libc-start.c:228)
==31871== 20 bytes in 1 blocks are still reachable in loss record 4 of 24
==31871==    at 0x4C28BED: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31871==    by 0x1FF567: _tor_malloc (util.c:144)
==31871==    by 0x1FF665: _tor_malloc_zero (util.c:170)
==31871==    by 0x1A72A7: connection_handle_write (connection.c:3751)
==31871==    by 0x11CF5D: conn_write_callback (main.c:739)
==31871==    by 0x52D8CCB: event_base_loop (in /usr/lib/x86_64-linux-gnu/libevent-2.0.so.5.1.7)
==31871==    by 0x11DB04: do_main_loop (main.c:1966)
==31871==    by 0x11F2ED: tor_main (main.c:2672)
==31871==    by 0x5F7BEAC: (below main) (libc-start.c:228)
==31301== 1,672 bytes in 1 blocks are still reachable in loss record 28 of 28
==31301==    at 0x4C28BED: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==31301==    by 0x1FF4E7: _tor_malloc (util.c:144)
==31301==    by 0x1FF5E5: _tor_malloc_zero (util.c:170)
==31301==    by 0x14769E: bw_array_new (rephist.c:1351)
==31301==    by 0x14778C: bw_arrays_init (rephist.c:1382)
==31301==    by 0x1480C4: rep_hist_init (rephist.c:202)
==31301==    by 0x11DF03: tor_init (main.c:2283)
==31301==    by 0x11F2BD: tor_main (main.c:2661)
==31301==    by 0x5F7BEAC: (below main) (libc-start.c:228)
==1852== 128 bytes in 1 blocks are still reachable in loss record 18 of 30
==1852==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==1852==    by 0x2125E7: _tor_malloc (util.c:144)
==1852==    by 0x213C25: _tor_malloc_zero (util.c:170)
==1852==    by 0x2277D7: tor_event_new (compat_libevent.c:124)
==1852==    by 0x2278B9: periodic_timer_new (compat_libevent.c:616)
==1852==    by 0x11A605: do_main_loop (main.c:1941)
==1852==    by 0x11A79C: tor_main (main.c:2668)
==1852==    by 0x5EFCC8C: (below main) (libc-start.c:228)
==1852== 18 bytes in 1 blocks are still reachable in loss record 5 of 30
==1852==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==1852==    by 0x5F59C61: strdup (strdup.c:43)
==1852==    by 0x211BED: _tor_strdup (util.c:241)
==1852==    by 0x19C8E3: get_short_version (config.c:628)
==1852==    by 0x19C96C: get_version (config.c:610)
==1852==    by 0x118EFC: tor_init (main.c:2280)
==1852==    by 0x11A782: tor_main (main.c:2661)
==1852==    by 0x5EFCC8C: (below main) (libc-start.c:228)

Child Tickets

Change History (9)

comment:1 Changed 6 years ago by arma

see my bug7029 branch

comment:2 Changed 6 years ago by arma

Status: newneeds_review

comment:3 Changed 6 years ago by arma

For future reference, here are two more I haven't resolved yet:

==1852== 1 bytes in 1 blocks are still reachable in loss record 1 of 30
==1852==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==1852==    by 0x2125E7: _tor_malloc (util.c:144)
==1852==    by 0x1EF679: configure_nameservers (dns.c:1206)
==1852==    by 0x11A24E: do_main_loop (main.c:1861)
==1852==    by 0x11A79C: tor_main (main.c:2668)
==1852==    by 0x5EFCC8C: (below main) (libc-start.c:228)

(Is there a proper way to free the result of an evdns_base_new()?)

==1852== 40 bytes in 1 blocks are still reachable in loss record 13 of 30
==1852==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==1852==    by 0x2125E7: _tor_malloc (util.c:144)
==1852==    by 0x213C25: _tor_malloc_zero (util.c:170)
==1852==    by 0x1FEE3E: tor_mutex_new (compat.c:2645)
==1852==    by 0x202049: get_n_open_sockets (compat.c:915)
==1852==    by 0x1B083D: retry_all_listeners (connection.c:894)
==1852==    by 0x1A1B70: options_act_reversible (config.c:954)
==1852==    by 0x1A1CFF: set_options (config.c:550)
==1852==    by 0x1A5DC8: options_init_from_string (config.c:3636)
==1852==    by 0x1A62AA: options_init_from_torrc (config.c:3493)
==1852==    by 0x1190AB: tor_init (main.c:2347)
==1852==    by 0x11A782: tor_main (main.c:2661)

(Looks like we'll want to add a compat_free_all() or the like, so we can reach into compat and free the mutex when we're done.)

comment:4 Changed 6 years ago by arma

We also don't free signal_events[] (from handle_signals()) at exit.

comment:5 Changed 6 years ago by arma

We also don't free launch_event (from dns_launch_correctness_checks()) at exit.

What's the right way to free evtimers that might still be remembered somewhere inside libevent?

comment:6 Changed 6 years ago by arma

Ha. I ran valgrind on moria1 for a bit, and found these gems:

==511== 16 bytes in 1 blocks are still reachable in loss record 4 of 24
==511==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==511==    by 0x2126F7: _tor_malloc (util.c:144)
==511==    by 0x20320E: smartlist_new (container.c:34)
==511==    by 0x14A271: rep_hist_add_buffer_stats (rephist.c:2399)
==511==    by 0x191321: circuit_free (circuitlist.c:627)
==511==    by 0x19154B: circuit_free_all (circuitlist.c:701)
==511==    by 0x118B04: tor_free_all (main.c:2456)
==511==    by 0x118C6B: tor_cleanup (main.c:2518)
==511==    by 0x1F9D68: consider_hibernation (hibernate.c:910)
==511==    by 0x11D2EE: second_elapsed_callback (main.c:1169)
==511==    by 0x52C9343: event_base_loop (in /usr/lib/libevent-1.4.so.2.1.3)
==511==    by 0x11A400: do_main_loop (main.c:1966)
==511== 2,048 bytes in 1 blocks are still reachable in loss record 21 of 24
==511==    at 0x4C245E2: realloc (vg_replace_malloc.c:525)
==511==    by 0x2124F7: _tor_realloc (util.c:218)
==511==    by 0x206B7F: smartlist_add (container.c:80)
==511==    by 0x191321: circuit_free (circuitlist.c:627)
==511==    by 0x19154B: circuit_free_all (circuitlist.c:701)
==511==    by 0x118B04: tor_free_all (main.c:2456)
==511==    by 0x118C6B: tor_cleanup (main.c:2518)
==511==    by 0x1F9D68: consider_hibernation (hibernate.c:910)
==511==    by 0x11D2EE: second_elapsed_callback (main.c:1169)
==511==    by 0x52C9343: event_base_loop (in /usr/lib/libevent-1.4.so.2.1.3)
==511==    by 0x11A400: do_main_loop (main.c:1966)
==511==    by 0x11A7BC: tor_main (main.c:2672)
==511== 3,120 bytes in 130 blocks are still reachable in loss record 22 of 24
==511==    at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==511==    by 0x2126F7: _tor_malloc (util.c:144)
==511==    by 0x213D35: _tor_malloc_zero (util.c:170)
==511==    by 0x14A224: rep_hist_add_buffer_stats (rephist.c:2394)
==511==    by 0x191321: circuit_free (circuitlist.c:627)
==511==    by 0x19154B: circuit_free_all (circuitlist.c:701)
==511==    by 0x118B04: tor_free_all (main.c:2456)
==511==    by 0x118C6B: tor_cleanup (main.c:2518)
==511==    by 0x1F9D68: consider_hibernation (hibernate.c:910)
==511==    by 0x11D2EE: second_elapsed_callback (main.c:1169)
==511==    by 0x52C9343: event_base_loop (in /usr/lib/libevent-1.4.so.2.1.3)
==511==    by 0x11A400: do_main_loop (main.c:1966)

It would appear that we have some dependencies on what order we free stuff on exit.

comment:7 Changed 6 years ago by nickm

Reading bug7029 --- it looks like some of these are from 0.2.3? I think it's worthwhile fixing the 0.2.3 ones in 0.2.3, since we might someday want to look for a memory leak or something that affects only 0.2.3, right?

(Is there a proper way to free the result of an evdns_base_new()?)

evdns_base_free().

What's the right way to free evtimers that might still be remembered somewhere inside libevent?

If we allocated them with event_new(), then event_free() should do just fine. It needs to be called before the event_base_free(), though. I believe Tor has wrappers for this.

comment:8 in reply to:  7 Changed 6 years ago by arma

Replying to nickm:

Reading bug7029 --- it looks like some of these are from 0.2.3? I think it's worthwhile fixing the 0.2.3 ones in 0.2.3, since we might someday want to look for a memory leak or something that affects only 0.2.3, right?

Maybe. Or we could get 0.2.3 out and focus on 0.2.4. I'm inclined to go the "don't ever look back" route.

comment:9 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

okay, merged into master then.

Note: See TracTickets for help on using tickets.