deadlock between addrman.h and util.cpp #9198

Last commit:-

HEAD is now at 44adf68... Merge #9159: [qa] Wait for specific block announcement in p2p-compactblocks

debug.log:-

bitcoin-qt: sync.cpp:125: void potential_deadlock_detected(const std::pair<void*, void*>&, const LockStack&, const LockStack&): Assertion `onlyMaybeDeadlock' failed.
2016-11-22 02:23:59.565410 POTENTIAL DEADLOCK DETECTED
2016-11-22 02:23:59.565483 Previous lock order was:
2016-11-22 02:23:59.565517  (1) *ppmutexOpenSSL[i]  util.cpp:123
2016-11-22 02:23:59.565555  (2) cs  addrman.h:561
2016-11-22 02:23:59.565589 Current lock order is:
2016-11-22 02:23:59.565617  (2) cs  addrman.h:348
2016-11-22 02:23:59.565648  (1) *ppmutexOpenSSL[i]  util.cpp:123

Looks like the first is coming from deserializing the addressbook using a CDataStream, which clears with zero_after_free_allocator, which uses the openssl lock callbacks.

I think the second comes from the usage of GetRandInt inside of CAddrman::Select().

I'm not sure how dangerous this is, but we should drop the need for openssl_cleanse in CDataStream, and instead let it take a templated param for its container, which is by default just a plain vector. Things that really need cleaning could request zero_after_free_allocator specifically.

Done here: https://github.com/theuni/bitcoin/commit/671c724716abdd69b9d253a01f8fec67a37ab7d7, though without taking a thorough look and adding cleansing back where it's actually necessary.

I'm still getting this fairly regularly from the latest master (76fcd9d5034143a5b041766552670d19f926097d).

2016-12-13 13:08:30.061882 dnsseed thread start
2016-12-13 13:08:30.061927 addcon thread start
2016-12-13 13:08:30.062153 init message: Done loading
2016-12-13 13:08:30.062183 msghand thread start
2016-12-13 13:08:30.062107 Loading addresses from 8 DNS seeds (could take a while)
2016-12-13 13:08:30.061891 net thread start
2016-12-13 13:08:30.062098 opencon thread start
2016-12-13 13:08:30.062386 GUI: Platform customization: "other" 
2016-12-13 13:08:30.406751 GUI: PaymentServer::LoadRootCAs: Loaded  169  root certificates 
2016-12-13 13:08:30.563296 trying connection 121.208.195.192:8333 lastseen=108.6days
2016-12-13 13:08:35.563796 connection to 121.208.195.192:8333 timeout
2016-12-13 13:08:35.563916 POTENTIAL DEADLOCK DETECTED
2016-12-13 13:08:35.563959 Previous lock order was:
2016-12-13 13:08:35.564007  (1) *ppmutexOpenSSL[i]  util.cpp:122 <----- locking_callback()
2016-12-13 13:08:35.564062  (2) cs  addrman.h:541 <----- Attempt()
2016-12-13 13:08:35.564108 Current lock order is:
2016-12-13 13:08:35.564145  (2) cs  addrman.h:346 (TRY) <----- Unserialize()
2016-12-13 13:08:35.564194  (1) *ppmutexOpenSSL[i]  util.cpp:122 <----- locking_callback()

The OpenSSL locks have been removed, so closing this issue.