seeder: Allow graceful program exit, plus many bug + code quality + UB fixes
Summary
This MR contains multiple fixes that I noticed needed to happen when reviewing !1820 (merged). It is organized so that each individual commit is relatively self-contained and can be reviewed in that way.
Note: it would be nice for this to be merged in non-squash mode, assuming it doesn't undergo significant "commit noise" as part of review.
Changes:
- Don't use
std::rand()
; it's not guaranteed thread-safe. Instead, use our ownFastRandomContext
in a thread-safe manner. - Refactor C-style polymorphism to use real C++ polymorphism for
CDnsThread
and its C-like companion,dns_opt_t
. - Fix potential UB/race condition when starting DNS threads
- Fix bug where if DNS server can't start, app is silent with no indication of error
- Refactor saving guts of ThreadDumper() to a separate function (used by a later commit)
- Resurrect the (long deleted)
CAddrDb::Skipped
function(s) (used by a later commit) - Fix potential C++ UB by avoiding passing pointers through
void *
- Refactor to use
std::thread
instead ofpthread
where appropriate - Respond to Ctrl-C,
SIGINT
,SIGTERM
, etc and gracefully exit, persisting data to db immediately before exit.- This issue was also something Sipa wanted to fix, see here: https://github.com/sipa/bitcoin-seeder/issues/76
- BUGFIX: Allows for app to not "lose" nodes when saving, at least on the final pre-exit save. Partially resolves: https://github.com/sipa/bitcoin-seeder/issues/108
- BUGFIX: Rarely, the
SaveAllToDisk()
function would crash the process iffopen()
fails (can happen if process runs out of file descriptors). This has already been merged to master in !1833 (merged) but is included in this MR as well in the refactor. - BUGFIX: It was possible for
CSeederNode
to leak file descriptors in some cases, eventually filling up the process fd table. This has been fixed as of !1833 (merged) but is also included here as part of the refactor.
Test Plan
ninja bitcoin-seeder check-bitcoin-seeder
- Run the seeder for a while e.g.:
$ ./src/seeder/bitcoin-seeder -port=53530 -host=myseed.me.com -ns=me.me.com -mbox='haha.gmail.com'
- You can hit the DNS server by using
nslookup
and settingserver 127.0.0.1
andset port=53530
and then you can query it by asking formyseed.me.com
(if you used-host=myseed.me.com
as above.. the host must match when you query).
- Periodically hit CTRL-C and observe it exit gracefully.
- Observe that on exit, the db is immediately persisted before exit.
- Optional:
- Try giving the DNS server a privileged port e.g. as non-root:
-port=53
and you should get an error on app startup. Previous to this MR it would just silently not work and the seeder would run without the DNS server but with no errors given to the user (not cool!). - Very optional: Try setting
DEBUG_THREAD_LIFETIMES = true
insrc/seeder/main.cpp
to see debug output showing how the threads gracefully exit. Note that the crawlers take a bit of time to fully exit because they may be busy connecting.
- Try giving the DNS server a privileged port e.g. as non-root:
Edited by Calin Culianu