aboutsummaryrefslogtreecommitdiff
path: root/test/functional/test_framework
Commit message (Collapse)AuthorAgeFilesLines
* Bugfix: QA: Run tests with UPnP disabledLuke Dashjr2019-09-241-0/+1
| | | | | | | Needed for builds configured with --enable-upnp-default Github-Pull: #16646 Rebased-From: b168dd30cf71ac176e271bc610b0b1a79ceaf075
* Add comments to Python ECDSA implementationJohn Newbery2019-06-261-16/+56
| | | | | Github-Pull: #15826 Rebased-From: b67978529ad02fc2665f2362418dc53db2e25e17
* Pure python ECPieter Wuille2019-06-211-204/+324
| | | | | | | | This removes the dependency on OpenSSL for the interaction tests, by providing a pure-Python toy implementation of secp256k1. Github-Pull: #15826 Rebased-From: 8c7b9324ca3f3ffb178bea56a96ea23f7e0383cb
* test: Add test for p2p_blocksonlyMarcoFalke2019-05-161-0/+8
| | | | | Github-Pull: #15990 Rebased-From: fa320de79faaca2b088fcbe7f76701faa9bff236
* test: Format predicate source as multiline on errorMarcoFalke2019-05-161-1/+1
| | | | | Github-Pull: #15990 Rebased-From: fa3872e7b4540857261aed948b94b6b2bfdbc3d1
* Merge #15419: qa: Always refresh cache to be out of ibdMarcoFalke2019-02-252-29/+28
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | fa2cdc9ac2 test: Simplify create_cache (MarcoFalke) fa25210d62 qa: Fix wallet_txn_doublespend issue (MarcoFalke) 1111aecbb5 qa: Always refresh stale cache to be out of ibd (MarcoFalke) fab0d85802 qa: Remove mocktime unless required (MarcoFalke) Pull request description: When starting a test, we are always in IBD because the timestamps on cached blocks are in the past. Usually, we solve that by generating a block at the beginning of the test. That is clumsy and might even lead to other problems such as #15360 and https://github.com/bitcoin/bitcoin/issues/14446#issuecomment-461926598 So fix that by getting rid of mocktime and always refreshing the last block of the cache when starting the test framework. Should fix #14446 Tree-SHA512: 6af09800f9c86131349a103af617a54551f5f3f3260d38e14e3f30fdd3d91a0feb0100c56cbb12eae4aeac5571ae4b530b16345cbb831d2670237b53351a22c1
| * test: Simplify create_cacheMarcoFalke2019-02-251-9/+6
| |
| * qa: Always refresh stale cache to be out of ibdMarcoFalke2019-02-191-2/+22
| |
| * qa: Remove mocktime unless requiredMarcoFalke2019-02-192-21/+3
| |
* | Merge #15404: [test] Remove -txindex to start nodesWladimir J. van der Laan2019-02-191-2/+2
|\ \ | |/ |/| | | | | | | | | | | | | | | 8e4b4f683a0b342cec24cd51b1e98433034ea2ea Address test todos by removing -txindex to nodes. Originally added when updating getrawtransaction to stop searching unspent utxos. (Amiti Uttarwar) Pull request description: Original todos added when removing getrawtransaction default behavior of searching unspent utxos. Tree-SHA512: d080953c3b0d2e5dca2265a15966dc25985a614c9cc86271ecd6276178ce428c85e262c24df92501695c32fed7beec0339b989f03cce91b57fb2efba201b7809
| * Address test todos by removing -txindex to nodes.Amiti Uttarwar2019-02-171-2/+2
| | | | | | | | Originally added when updating getrawtransaction to stop searching unspent utxos.
* | Merge #15415: [test] functional: allow custom cwd, use tmpdir as defaultMarcoFalke2019-02-192-3/+9
|\ \ | |/ |/| | | | | | | | | | | | | | | e3e1a5631e [test] functional: set cwd of nodes to tmpdir (Sjors Provoost) Pull request description: Any process launched by bitcoind will have `self.datadir` as its `cwd`. Tree-SHA512: 0b311643bb96c7dc2f693774620173243b3add40bf373284695af2f0071823b23485289fd2ffe152b7f63e0bfe989b16720077cfc2ce33905f9b8e7f2630f3c0
| * [test] functional: set cwd of nodes to tmpdirSjors Provoost2019-02-192-3/+9
| |
* | Descriptor checksumPieter Wuille2019-02-151-0/+55
| |
* | Merge #15383: [rpc] mining: Omit uninitialized currentblockweight, ↵MarcoFalke2019-02-151-1/+5
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | currentblocktx fa178a6385 [rpc] mining: Omit uninitialized currentblockweight, currentblocktx (MarcoFalke) Pull request description: Previously we'd report "0", which could be mistaken for a valid number. E.g. the number of transactions is 0 or the block weight is 0, whatever that means. Tree-SHA512: ee94ab203a329e272211b726f4c23edec4b09c650ec363b77fd59ad9264165d73064f78ebb9e11b5c2c543b73c157752410a307655560531c7d5444d203aa0ea
| * [rpc] mining: Omit uninitialized currentblockweight, currentblocktxMarcoFalke2019-02-121-1/+5
| |
* | Merge #15238: [QA] remove some magic mining constants in functional testsMarcoFalke2019-02-121-1/+3
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | b651ef7e1c submitheader: more directly test missing prev block header (Gregory Sanders) 1e7f741745 remove some magic mining constants in functional tests (Gregory Sanders) Pull request description: The fewer magic numbers the better. Also more directly tested a `submitheader` case of bad previous blockhash. Tree-SHA512: 52b01a6aa199fa909eea4e9e84409a901933e545724e33149cc4132c82168199fd678809b6d94d95c9ff6ad02238a9552363620d13b8beaa5d4b67ade9ef425c
| * remove some magic mining constants in functional testsGregory Sanders2019-01-241-1/+3
| |
* | qa: Drop RPC connection if --usecliJoão Barbosa2019-02-061-3/+6
| |
* | Merge #14519: tests: add utility to easily profile node performance with perfMarcoFalke2019-02-052-3/+111
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 13782b8ba8 docs: add perf section to developer docs (James O'Beirne) 58180b5fd4 tests: add utility to easily profile node performance with perf (James O'Beirne) Pull request description: Adds a context manager to easily (and selectively) profile node performance during functional test execution using `perf`. While writing some tests, I encountered some odd bitcoind slowness. I wrote up a utility (`TestNode.profile_with_perf`) that generates performance diagnostics for a node by running `perf` during the execution of a particular region of test code. `perf` usage is detailed in the excellent (and sadly unmerged) https://github.com/bitcoin/bitcoin/pull/12649; all due props to @eklitzke. ### Example ```python with node.profile_with_perf("large-msgs"): for i in range(200): node.p2p.send_message(some_large_msg) node.p2p.sync_with_ping() ``` This generates a perf data file in the test node's datadir (`/tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data`). Running `perf report` generates nice output about where the node spent most of its time while running that part of the test: ```bash $ perf report -i /tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data --stdio \ | c++filt \ | less # To display the perf.data header info, please use --header/--header-only options. # # # Total Lost Samples: 0 # # Samples: 135 of event 'cycles:pp' # Event count (approx.): 1458205679493582 # # Children Self Command Shared Object Symbol # ........ ........ ............... ................... ........................................................................................................................................................................................................................................................................ # 70.14% 0.00% bitcoin-net bitcoind [.] CNode::ReceiveMsgBytes(char const*, unsigned int, bool&) | ---CNode::ReceiveMsgBytes(char const*, unsigned int, bool&) 70.14% 0.00% bitcoin-net bitcoind [.] CNetMessage::readData(char const*, unsigned int) | ---CNetMessage::readData(char const*, unsigned int) CNode::ReceiveMsgBytes(char const*, unsigned int, bool&) 35.52% 0.00% bitcoin-net bitcoind [.] std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&) | ---std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&) CNetMessage::readData(char const*, unsigned int) CNode::ReceiveMsgBytes(char const*, unsigned int, bool&) ... ``` Tree-SHA512: 9ac4ceaa88818d5eca00994e8e3c8ad42ae019550d6583972a0a4f7b0c4f61032e3d0c476b4ae58756bc5eb8f8015a19a7fc26c095bd588f31d49a37ed0c6b3e
| * | tests: add utility to easily profile node performance with perfJames O'Beirne2019-01-222-3/+111
| | | | | | | | | | | | | | | | | | | | | | | | Introduces `TestNode.profile_with_perf()` context manager which samples node execution to produce profiling data. Also introduces a test framework flag, `--perf`, which will run perf on all nodes for the duration of a given test.
* | | tests: unify RPC argument to cli argument conversion and handle dicts and listsAndrew Chow2019-01-311-2/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | When running tests with --usecli, unify the conversion from argument objects to strings using a new function arg_to_cli(). This fixes boolean arguments when using named arguments. Also use json.dumps() to get the string values for arguments that are dicts and lists so that bitcoind's JSON parser does not become confused.
* | | Merge #13926: [Tools] bitcoin-wallet - a tool for creating and managing ↵MarcoFalke2019-01-311-0/+1
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | wallets offline 3c3e31c3a4 [tests] Add wallet-tool test (João Barbosa) 49d2374acf [tools] Add wallet inspection and modification tool (Jonas Schnelli) Pull request description: Adds an offline tool `bitcoin-wallet-tool` for wallet creation and maintenance. Currently this tool can create a new wallet file, display information on an existing wallet, and run the salvage and zapwallettxes maintenance tasks on an existing wallet. It can later be extended to support other common wallet maintenance tasks. Doing wallet maintenance tasks in an offline tool makes much more sense (and is potentially safer) than having to spin up a full node. Tree-SHA512: 75a28b8a58858d9d76c7532db40eacdefc5714ea5aab536fb1dc9756e2f7d750d69d68d59c50a68e633ce38fb5b8c3e3d4880db30fe01561e07ce58d42bceb2b
| * | | [tests] Add wallet-tool testJoão Barbosa2019-01-301-0/+1
| | |/ | |/| | | | | | | | | | | | | Original tests by João Barbosa <[email protected]> Additional contribution by John Newbery <[email protected]>
* / | qa: Add tests for invalid message headersMarcoFalke2019-01-241-3/+3
|/ /
* | Merge #15108: [tests] tidy up wallet_importmulti.pyMarcoFalke2019-01-091-0/+99
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 2d5f1ea2e3 [tests] move wallet util functions to wallet_util.py (John Newbery) 6be64ef02c [tests] tidy up wallet_importmulti.py (John Newbery) Pull request description: Cherry picks un-merged commits from #14952, which "fixes review comments from @ryanofsky here: https://github.com/bitcoin/bitcoin/pull/14886#pullrequestreview-183772779" Tree-SHA512: 5f389196b0140d013a533d500f1812786a3a5cfb65980e13eaeacc459fddb55f43d05da3ab5e7cc8c997f26c0b667eed081ab6de2d125e631c70a7dd4c06e350
| * | [tests] move wallet util functions to wallet_util.pyJohn Newbery2019-01-041-0/+99
| |/ | | | | | | | | | | | | | | | | Adds a new wallet_util.py module and moves generic helper functions there: - get_key - get_multisig - test_address
* | Merge #15059: test: Add basic test for BIP34Wladimir J. van der Laan2019-01-081-1/+2
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | fab17e8272f5f70213f186809479ee7a75898b1d test: Add basic test for BIP34 (MarcoFalke) Pull request description: BIP34 was disabled for testing, which explains why it had no test. Fix that by enabling it and adding a test. Tree-SHA512: 9cb5702d474117ce6420226eb93ee09d6fb5fc856fabc8b67abe56a088cd727674e0e5462000e1afa83b911374036f90abdbdde56a8c236a75572ed47e10a00f
| * test: Add basic test for BIP34MarcoFalke2018-12-291-1/+2
| |
* | Merge #14457: test: add invalid tx templates for use in functional testsWladimir J. van der Laan2019-01-021-0/+2
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 59e387705c7e55ec40400301346354fa2d0c613f test: add invalid tx templates for use in functional tests (James O'Beirne) Pull request description: This change adds a list of `CTransaction`-generating templates which each correspond to a specific type of invalid transaction. We then use this list to test for a wider variety of invalid tx types in `p2p_invalid_tx.py` and `feature_block.py`. Consolidating all invalid tx types will allow us to more easily cover all tx reject cases from a variety of tests without repeating ourselves. Validation logic doesn't differ much between mempool and block acceptance, but there *is* a difference and we should be sure we're testing both comprehensively. Right now, I've only added templates covering the tx reject types listed below but if this approach seems worthwhile I will expand the list to be fully comprehensive. ``` bad-txns-in-belowout bad-txns-inputs-duplicate bad-txns-too-many-sigops bad-txns-vin-empty bad-txns-vout-empty bad-txns-vout-negative ``` Tree-SHA512: 05407f4a953fbd7c44c08bb49bb989cefd39a2b05ea00f5b3c92197a3f05e1b302f789e33832445734220e1c333d133aba385740b77b84139b170c583471ce20
| * | test: add invalid tx templates for use in functional testsJames O'Beirne2018-11-271-0/+2
| | | | | | | | | | | | | | | Add templates for easily constructing different kinds of invalid transactions and use them in feature_block and p2p_invalid_tx.
* | | Merge #15026: [test] Rename rpc_timewait to rpc_timeoutMarcoFalke2018-12-291-3/+14
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 4999992c34 whitespace: Split ~300 char line into multiple ones (MarcoFalke) fa71b38168 scripted-diff: Rename rpc_timewait to rpc_timeout (MarcoFalke) fa3e5786d0 scripted-diff: Remove unused 'split' parameter to setup_network (MarcoFalke) Pull request description: This is a bugfix, since wallet_dump currently uses the wrong name: https://github.com/bitcoin/bitcoin/blob/18857b4c4034af54e7ad3cbd78ff6f87f4f22567/test/functional/wallet_dump.py#L89-L92 Rename all to the same name with a scripted diff (and some unrelated cleanups). Tree-SHA512: 338ddd20dae12e6cf7aa7adbcfb239cf648017a1572b373f8431fecb184bd2a65492846d81e75a023864d9e41c94afb53044c16b79651a5937d34a5a6b772f81
| * | | whitespace: Split ~300 char line into multiple onesMarcoFalke2018-12-221-1/+12
| | | |
| * | | scripted-diff: Rename rpc_timewait to rpc_timeoutMarcoFalke2018-12-221-3/+3
| | |/ | |/| | | | | | | | | | | | | -BEGIN VERIFY SCRIPT- sed -i -e 's/self.rpc_timewait/self.rpc_timeout/g' $(git grep -l self.rpc_timewait) -END VERIFY SCRIPT-
* | | Merge #14738: Tests: Fix running wallet_listtransactions.py individually ↵MarcoFalke2018-12-291-17/+6
|\ \ \ | |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | through test_runner.py 2474de0265 Fix running individually through test_runner.py, as suggested by @MarcoFalke (#14732) (Kristaps Kaupe) Pull request description: As suggested by @MarcoFalke. Resolves #14732. Tree-SHA512: b4e400ba06075e218dbd97d0390845f1c55be42a2b6fd70513381318cfc2693473ba1d0f9d7f379a96939d1960b53801fad7c02e06bddc50c5a835ad024c37ef
| * | Fix running individually through test_runner.py, as suggested by @MarcoFalke ↵Kristaps Kaupe2018-11-161-17/+6
| | | | | | | | | | | | (#14732)
* | | Merge #14951: Revert "tests: Support calling add_nodes more than once"MarcoFalke2018-12-141-3/+17
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | fa4b8c90d3 test: add_nodes can only be called once after set_test_params (MarcoFalke) faa831102a Revert "tests: Support calling add_nodes more than once" (MarcoFalke) Pull request description: Writing tests should be straightforward and with little side-effects as possible. I don't see how this is needed and can not be achieved with `self.num_nodes` (and `self.extra_args` et al.) Tree-SHA512: 83a67f2cba9d97e21d80847ff405a4633fcb0d5674486efa57ee1813e46efe8709ae0fb462b8339a01ebeca5c4f2d29ecb1807d648b8fd9ee8ce336b08d580a8
| * | | test: add_nodes can only be called once after set_test_paramsMarcoFalke2018-12-131-2/+17
| | | |
| * | | Revert "tests: Support calling add_nodes more than once"MarcoFalke2018-12-131-2/+1
| | | | | | | | | | | | | | | | This reverts commit 98a1846b00d9c3076d6dcd96244fae6f923e26a0.
* | | | Merge #14805: tests: Support calling add_nodes more than onceWladimir J. van der Laan2018-12-131-1/+2
|\| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 98a1846b00d9c3076d6dcd96244fae6f923e26a0 tests: Support calling add_nodes more than once (Steven Roose) Pull request description: Ran into this while writing [a multi-chain test for Elements](https://github.com/ElementsProject/elements/pull/458) where I call this method more than once. Tree-SHA512: f2d698fcb560552aa5d81a4c3fbf40b7269b228b34d85a118291649ef83f8c0a30cd82a28d418237b55893bcecd538046b704e64a4d8a41f2c0aef8033dc83e5
| * | | tests: Support calling add_nodes more than onceSteven Roose2018-11-211-1/+2
| | | |
* | | | Merge #14926: test: consensus: Check that final transactions are validWladimir J. van der Laan2018-12-131-0/+2
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | aaaa8eb1edba2a28916d5da6001d421c1b1b253b test: consensus: Check that final transactions are valid (MarcoFalke) fae3617d79deee73dd375dc3ea5f4204a74420c5 test: Correctly deserialize without witness (MarcoFalke) Pull request description: There is no check that checks that final transactions are valid, i.e. the consensus rules could be changed (accidentally) with none of the tests failing. Tree-SHA512: 48f4c24bfcc525ddbc1bfe8c37131953b464823428c1f7a278ba6d98b98827b6b84a8eb2b33396bfb5b8cc4012b7cc1cd771637f405ea20beddae001c22aa290
| * | | | test: Correctly deserialize without witnessMarcoFalke2018-12-111-0/+2
| | | | |
* | | | | Merge #14884: Travis: enforce Python 3.4 support through linterWladimir J. van der Laan2018-12-131-0/+4
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 31926ee8cfc73501524dfa0fef2ccbaa786d6a00 [test] functional framework: add CScript hex() for Python 3.4 (Sjors Provoost) 74ce32683199b987e45eb16f0320ae392ff10edc [test] Travis: enforce Python 3.4 support in functional tests (Sjors Provoost) Pull request description: The minimum supported version of Python is 3.4 according to [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md). This PR makes the Travis linter use this version in order to catch accidental use of modern syntax. Tree-SHA512: 71b2c102be72b135a8ba049378d66875760f20a04a657102a399240c5c2b2ddbdfa7d5ab4c0c0242ecc3259e0ee8eb2273f331bc5eb824f4ae4c3cc58aea37ac
| * | | | | [test] functional framework: add CScript hex() for Python 3.4Sjors Provoost2018-12-121-0/+4
| |/ / / / | | | | | | | | | | | | | | | | | | | | test/functional/wallet_importmulti.py failed with: AttributeError: 'CScript' object has no attribute 'hex'
* / / / / Compare to None with is/is notDaniel Ingram2018-12-102-5/+5
|/ / / /
* | | | Merge #14788: tests: Possible fix the permission error when the tests open ↵MarcoFalke2018-12-071-1/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | the cookie file d6b3790d1a tests: check readability of cookie file (Chun Kuan Lee) Pull request description: This PR would wait until the `.cookie` file is readable Possible fix no. 5 `PermissionError` in #14446 Tree-SHA512: e7055c7ca26a6eadbbe19e4eef08ffee61cd17de79b30af2f0d090f0ad81ca24815e3c7e034e5e30d47c580bb0b221b3955e9ff2fcec2274fbf7b9232ab0cdc7
| * | | | tests: check readability of cookie fileChun Kuan Lee2018-11-241-1/+1
| |/ / /
* | | | Merge #14670: http: Fix HTTP server shutdownWladimir J. van der Laan2018-12-062-6/+6
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 28479f926f21f2a91bec5a06671c60e5b0c55532 qa: Test bitcond shutdown (João Barbosa) 8d3f46ec3938e2ba17654fecacd1d2629f9915fd http: Remove timeout to exit event loop (João Barbosa) e98a9eede2fb48ff33a020acc888cbcd83e24bbf http: Remove unnecessary event_base_loopexit call (João Barbosa) 6b13580f4e3842c11abd9b8bee7255fb2472b6fe http: Unlisten sockets after all workers quit (João Barbosa) 18e968581697078c36a3c3818f8906cf134ccadd http: Send "Connection: close" header if shutdown is requested (João Barbosa) 02e1e4eff6cda0bfc24b455a7c1583394cbff6eb rpc: Add wait argument to stop (João Barbosa) Pull request description: Fixes #11777. Reverts #11006. Replaces #13501. With this change the HTTP server will exit gracefully, meaning that all requests will finish processing and sending the response, even if this means to wait more than 2 seconds (current time allowed to exit the event loop). Another small change is that connections are accepted even when the server is stopping, but HTTP requests are rejected. This can be improved later, especially if chunked replies are implemented. Briefly, before this PR, this is the order or events when a request arrives (RPC `stop`): 1. `bufferevent_disable(..., EV_READ)` 2. `StartShutdown()` 3. `evhttp_del_accept_socket(...)` 4. `ThreadHTTP` terminates (event loop exits) because there are no active or pending events thanks to 1. and 3. 5. client doesn't get the response thanks to 4. This can be verified by applying ```diff // Event loop will exit after current HTTP requests have been handled, so // this reply will get back to the client. StartShutdown(); + MilliSleep(2000); return "Bitcoin server stopping"; } ``` and checking the log output: ``` Received a POST request for / from 127.0.0.1:62443 ThreadRPCServer method=stop user=__cookie__ Interrupting HTTP server ** Exited http event loop Interrupting HTTP RPC server Interrupting RPC tor: Thread interrupt Shutdown: In progress... torcontrol thread exit Stopping HTTP RPC server addcon thread exit opencon thread exit Unregistering HTTP handler for / (exactmatch 1) Unregistering HTTP handler for /wallet/ (exactmatch 0) Stopping RPC RPC stopped. Stopping HTTP server Waiting for HTTP worker threads to exit msghand thread exit net thread exit ... sleep 2 seconds ... Waiting for HTTP event thread to exit Stopped HTTP server ``` For this reason point 3. is moved right after all HTTP workers quit. In that moment HTTP replies are queued in the event loop which keeps spinning util all connections are closed. In order to trigger the server side close with keep alive connections (implicit in HTTP/1.1) the header `Connection: close` is sent if shutdown was requested. This can be tested by ``` bitcoind -regtest nc localhost 18443 POST / HTTP/1.1 Authorization: Basic ... Content-Type: application/json Content-Length: 44 {"jsonrpc": "2.0","method":"stop","id":123} ``` Summing up, this PR: - removes explicit event loop exit — event loop exits once there are no active or pending events - changes the moment the listening sockets are removed — explained above - sends header `Connection: close` on active requests when shutdown was requested which is relevant when it's a persistent connection (default in HTTP 1.1) — libevent is aware of this header and closes the connection gracefully - removes event loop explicit break after 2 seconds timeout Tree-SHA512: 4dac1e86abe388697c1e2dedbf31fb36a394cfafe5e64eadbf6ed01d829542785a8c3b91d1ab680d3f03f912d14fc87176428041141441d25dcb6c98a1e069d8
| * | | | qa: Test bitcond shutdownJoão Barbosa2018-11-232-6/+6
| |/ / /