Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Global cleanups #2420

Merged
merged 1 commit into from Apr 2, 2013

Conversation

Projects
None yet
5 participants
Owner

sipa commented Mar 29, 2013

Clean up all known global datastructures at shutdown.

This should make leak detection much easier.

Typical valgrind output after this:
==5897== definitely lost: 0 bytes in 0 blocks
==5897== indirectly lost: 0 bytes in 0 blocks
==5897== possibly lost: 1,903 bytes in 9 blocks
==5897== still reachable: 194,073 bytes in 2,929 blocks

(almost all of the reachable stuff is RPC/asio/boostthreads)

Clean up global datastructures at shutdown.
This should make detecting leaks much easier.
Contributor

jgarzik commented Mar 29, 2013

Nice!

FWIW, In my own programs, I usually add a "--free" command line option, which optionally cleans up on exit (slowing down exit), rather than the rapid exit(0). Similarly, since freeing the block index and other numerous structures is a lot of pointless work at shutdown for 99.9% of users, I would suggest something similar for bitcoind.

Owner

sipa commented Mar 29, 2013

I certainly don't notice it, but if you want it optional, sure, that can be arranged.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/3427517d507a938074a50fa8ea6dfe3d13bef357 for binaries and test log.
This is an automated test script which runs test cases on each commit every time is updated.
It, however, dies sometimes and fails to test properly, if you are waiting on a test, please check timestamps and if the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
and contact BlueMatt on freenode if something looks broken.

Diapolo commented Apr 2, 2013

How much extra time does that cleanup take? If it's not a huge increase in shutdown time, why make it optional?

Contributor

gavinandresen commented Apr 2, 2013

Very nice.

We can make this optional later, if needed, I'm going to pull now.

gavinandresen added a commit that referenced this pull request Apr 2, 2013

@gavinandresen gavinandresen merged commit d8aae1c into bitcoin:master Apr 2, 2013

@sipa sipa referenced this pull request Apr 2, 2013

Merged

Limited mapAlreadyAskedFor #2423

@sipa sipa deleted the sipa:globclean branch May 3, 2013

laudney pushed a commit to reddcoin-project/reddcoin that referenced this pull request Mar 19, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment