Skip to content

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 own FastRandomContext 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 of pthread where appropriate
  • Respond to Ctrl-C, SIGINT, SIGTERM, etc and gracefully exit, persisting data to db immediately before exit.
  • BUGFIX: Rarely, the SaveAllToDisk() function would crash the process if fopen() 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

  1. ninja bitcoin-seeder check-bitcoin-seeder
  2. 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 setting server 127.0.0.1 and set port=53530 and then you can query it by asking for myseed.me.com (if you used -host=myseed.me.com as above.. the host must match when you query).
  1. Periodically hit CTRL-C and observe it exit gracefully.
  2. Observe that on exit, the db is immediately persisted before exit.
  3. 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 in src/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.
Edited by Calin Culianu

Merge request reports