-
Notifications
You must be signed in to change notification settings - Fork 178
mv valgrind cleanup
- development started August 27, 2013
Valgrind is an excellent program diagnostic tool (www.valgrind.org). Unfortunately, it is easy to let a program get into a condition such that valgrind generates more "noise" than actionable information. The Riak branch of leveldb was in such a condition. Some of the noise was from the original Google implementation. Some of the noise was from programmer laziness in the Riak modifications. This branch cleans up all the noise and addresses an actual edge case memory leak detected by valgrind. The memory leak joined the code base with Riak 1.4.
Valgrind was used with each of the unit test programs ("make check"). The branch cleans up the leveldb code and/or the unit test as appropriate to achieve a clean valgrind execution of the following command:
valgrind --leak-check=full --show-reachable=yes ./unit_test_program
"unit_test_program" is a placeholder for each of the Google unit test programs created by "make check". Warning: valgrind makes some programs run for a long, long time. Example: "db_bench" requires over an hour under valgrind. And valgrind limits everything to a single CPU. Pick a machine with a fast clock rate over one with tons of CPUs. The unit tests addressed by this branch are:
arena_test coding_test c_test env_test log_test table_test write_batch_test bloom_test corruption_test dbformat_test filename_test memenv_test version_edit_test cache_test crc32c_test db_test filter_block_test skiplist_test version_set_test
Any thread created with pthread_create allocates a block of memory that is later used to return the exit value of the thread. valgrind reports that exit value memory as a leak unless the code calls either pthread_join() or pthread_detach(). None of the original Google leveldb or unit test code included either call. Likewise, none of the Riak changes included either call.
Many of the unit tests utilize the Env::StartThread call to create a thread. Previously this call had a "void" return value. It now returns the pthread thread id. The Env::StartThread declaration changed in include/leveldb/env.h. Also the EnvPosix::StartThread declaration and implementation changed within util/env_posix.cc. The unit tests utilizing the call now save the return value and immediate call pthread_detach() with that value. Memory leak fixed.
Google's original PosixEnv::Schedule() routine in util/env_posix.cc called pthread_create directly instead of calling Env::StartThread. Riak's expansion of this routine to 4 background threads followed the same design. The pthread thread id has always been saved in both cases, but never used. The thread id is now used in the PosixEnv::~PosixEnv destructor to properly close all threads via pthread_join().
Google's leveldb implementation lacks an API call for shutting down its background compaction thread.