Memory leakage from the global HASHTABLE

[Fugoes]

When I compile the following program, and test it with the valgrind tool, it shows memory leakage (the valgrind's log is attached following the program).

use std::sync::Arc;
use std::thread::spawn;

use parking_lot::{Condvar, Mutex};

struct Barrier {
    cond: Condvar,
    n: Mutex<u32>,
}

impl Barrier {
    fn new(n: u32) -> Self { Self { cond: Condvar::new(), n: Mutex::new(n) } }

    fn arrive(&self) {
        let mut guard = self.n.lock();
        let flag = *guard;
        if *guard == 0 { panic!() };
        *guard -= 1;
        drop(guard);
        if flag == 1 { self.cond.notify_all(); };
    }

    fn wait_all(&self) {
        let mut guard = self.n.lock();
        while *guard != 0 { self.cond.wait(&mut guard) };
    }
}

fn main() {
    let mut hs = vec![];
    let mut barriers = vec![];
    for _ in 0..128 { barriers.push(Arc::new(Barrier::new(16))); };
    for _ in 0..16 {
        let barriers = barriers.clone();
        hs.push(spawn(move || {
            for barrier in barriers.iter() { barrier.arrive(); };
        }));
    };
    for barrier in barriers.iter() { barrier.wait_all(); };
    for h in hs.into_iter() { let _ = h.join(); };
}

# valgrind --leak-check=full --show-leak-kinds=all ./test
==25451== Memcheck, a memory error detector
==25451== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==25451== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==25451== Command: ./test
==25451== 
==25451== 
==25451== HEAP SUMMARY:
==25451==     in use at exit: 1,056 bytes in 2 blocks
==25451==   total heap usage: 317 allocs, 315 frees, 36,680 bytes allocated
==25451== 
==25451== 32 bytes in 1 blocks are still reachable in loss record 1 of 2
==25451==    at 0x483577F: malloc (vg_replace_malloc.c:299)
==25451==    by 0x1106F6: parking_lot_core::parking_lot::HashTable::new (in /home/someone/test)
==25451==    by 0x110A6F: parking_lot_core::parking_lot::create_hashtable (in /home/someone/test)
==25451==    by 0x110895: parking_lot_core::parking_lot::ThreadData::new (in /home/someone/test)
==25451==    by 0x110E4F: _ZN3std6thread5local4fast12Key$LT$T$GT$14try_initialize17h21e3b1298e9bdf89E.llvm.2676389730810592219 (in /home/someone/test)
==25451==    by 0x10F454: parking_lot::condvar::Condvar::wait_until_internal (in /home/someone/test)
==25451==    by 0x10DE73: yaay_example::main (in /home/someone/test)
==25451==    by 0x10D392: std::rt::lang_start::{{closure}} (in /home/someone/test)
==25451==    by 0x117C92: {{closure}} (rt.rs:48)
==25451==    by 0x117C92: std::panicking::try::do_call (panicking.rs:287)
==25451==    by 0x11A0B9: __rust_maybe_catch_panic (lib.rs:78)
==25451==    by 0x1186FC: try<i32,closure-0> (panicking.rs:265)
==25451==    by 0x1186FC: catch_unwind<closure-0,i32> (panic.rs:396)
==25451==    by 0x1186FC: std::rt::lang_start_internal (rt.rs:47)
==25451==    by 0x10E111: main (in /home/someone/test)
==25451== 
==25451== 1,024 bytes in 1 blocks are still reachable in loss record 2 of 2
==25451==    at 0x4837EC3: memalign (vg_replace_malloc.c:898)
==25451==    by 0x4837FF0: posix_memalign (vg_replace_malloc.c:1062)
==25451==    by 0x1174B9: aligned_malloc (alloc.rs:89)
==25451==    by 0x1174B9: alloc (alloc.rs:22)
==25451==    by 0x1174B9: __rdl_alloc (alloc.rs:239)
==25451==    by 0x1105E3: parking_lot_core::parking_lot::HashTable::new (in /home/someone/test)
==25451==    by 0x110A6F: parking_lot_core::parking_lot::create_hashtable (in /home/someone/test)
==25451==    by 0x110895: parking_lot_core::parking_lot::ThreadData::new (in /home/someone/test)
==25451==    by 0x110E4F: _ZN3std6thread5local4fast12Key$LT$T$GT$14try_initialize17h21e3b1298e9bdf89E.llvm.2676389730810592219 (in /home/someone/test)
==25451==    by 0x10F454: parking_lot::condvar::Condvar::wait_until_internal (in /home/someone/test)
==25451==    by 0x10DE73: yaay_example::main (in /home/someone/test)
==25451==    by 0x10D392: std::rt::lang_start::{{closure}} (in /home/someone/test)
==25451==    by 0x117C92: {{closure}} (rt.rs:48)
==25451==    by 0x117C92: std::panicking::try::do_call (panicking.rs:287)
==25451==    by 0x11A0B9: __rust_maybe_catch_panic (lib.rs:78)
==25451== 
==25451== LEAK SUMMARY:
==25451==    definitely lost: 0 bytes in 0 blocks
==25451==    indirectly lost: 0 bytes in 0 blocks
==25451==      possibly lost: 0 bytes in 0 blocks
==25451==    still reachable: 1,056 bytes in 2 blocks
==25451==         suppressed: 0 bytes in 0 blocks
==25451== 
==25451== For counts of detected and suppressed errors, rerun with: -v
==25451== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

It seems the parking_lot_core::parking_lot::HASHTABLE pointer does not get dropped after the program exit. Adding a function to do the cleanup would be handy.

Adding the following function to parking_lot/core/parking_lot.rs and export it to library's users would fix the problem, though users need to call the function manually at proper time.

/// Drop the global `HASHTABLE` instance.
pub unsafe fn cleanup() {
    let table = HASHTABLE.swap(null_mut(), Ordering::SeqCst);
    if !table.is_null() {
        drop(Box::from_raw(table));
    };
}

parking_lot_core provides a low-level API for creating new synchronization primitives. Doing low-level programming in rust envoles many unsafe blocks, and memory leakage might occur. This patch would ease the debug process for memory leakage when using tools like valgrind.

[faern]

Calling said cleanup function at the proper time would be extremely hard and error prone. Since there is a single global HASHTABLE in the process. How would you know all your dependencies are done using parking_lot?

It's also not that simple. The HASHTABLE has references to previous hashtables. When the size of the table is increased the old tables are kept on the heap since we can't guarantee all other threads are done using it yet. The new table keeps a pointer to the old one just to please tools like valgrind. As such the design of parking_lot_core includes "leaking" a number of HASHTABLEs during the process' runtime. However, since the size of the table only grows, and the size is determined by the number of threads in the process, this is usually not a large leak at all.

[Fugoes]

Thanks for the reply! Sure this is not a large leak. And after more trying I found that rust basically does not properly support 'singleton destructor pattern'.