28479f926f qa: Test bitcond shutdown (João Barbosa)
8d3f46ec39 http: Remove timeout to exit event loop (João Barbosa)
e98a9eede2 http: Remove unnecessary event_base_loopexit call (João Barbosa)
6b13580f4e http: Unlisten sockets after all workers quit (João Barbosa)
18e9685816 http: Send "Connection: close" header if shutdown is requested (João Barbosa)
02e1e4eff6 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
This requires a small changes to a few tests, but means that
deterministic addresses will always be imported (unless setup_nodes
behaviour is explicitly overridden).
a2a04a5abb Bugfix: Only run bitcoin-tx tests when bitcoin-tx is enabled (Luke Dashjr)
92af71cea9 configure: Make it possible to build only one of bitcoin-cli or bitcoin-tx (Luke Dashjr)
Pull request description:
Includes #5618 (which the reasons for rejecting no longer hold true)
Tree-SHA512: f30a8e4a2f70166b7cabef77c4674163b3a9da14c6a547d34f00d1056a19bf4d23e22851eea726fad2afc8735d5473ae91122c770b65ac3886663dc20e2c5b70
3fc20632a3 qt: Set BLOCK_CHAIN_SIZE = 220 (DrahtBot)
2b6a2f4a28 Regenerate manpages (DrahtBot)
eb7daf4d60 Update copyright headers to 2018 (DrahtBot)
Pull request description:
Some trivial maintenance to avoid having to do it again after the 0.17 branch off.
(The scripts to do this are in `./contrib/`)
Tree-SHA512: 16b2af45e0351b1c691c5311d48025dc6828079e98c2aa2e600dc5910ee8aa01858ca6c356538150dc46fe14c8819ed8ec8e4ec9a0f682b9950dd41bc50518fa
c9cce0a Tests: Add Metaclass for BitcoinTestFramework (Will Ayd)
Pull request description:
BitcoinTestFramework instructs developers in its docstring to override
`set_test_params` and `run_test` in subclasses while being sure NOT to
override `__init__` and `main` . This change adds a metaclass to ensure
that developers adhere to that protocol, raising a ``TypeError`` in
instances where they have not.
closes#12835
Tree-SHA512: 5a47a7ead1f18361138cad4374747c4a8f29d25506f7b2c2a8c1c966a0b65e5ccf7317f9a078df8680fdab5d3fb71fee46a159c9f381878a3683c1e9f874abbe
e36a0c0 [qa] Ensure bitcoind processes are cleaned up when tests end (Suhas Daftuar)
Pull request description:
When tests fail (such as due to a bug in the test, race condition, etc), it's possible that we could follow code paths that bypass our normal node shutdown that occurs in `TestNode.stop_node`. Add a destructor to `TestNode` that cleans this up.
Tree-SHA512: 72e04bc21462ebd0cb346fd1fe0540da454acfbad41923a0b06ea2317e9045b68e58f9adb02d8200891aca89a9d03a022eb72282aeb31a3b3afe7c6843a4b450
BitcoinTestFramework instructs developers in its docstring to override
`set_test_params` and `run_test` in subclasses while being sure NOT to
override `__init__` and `main` . This change adds a metaclass to ensure
that developers adhere to that protocol, raising a ``TypeError`` in
instances where they have not.
closes#12835
c8330d4 qa: Use node.datadir instead of tmpdir in test framework (MarcoFalke)
Pull request description:
Commit c53c9831ee introduced the utility function `get_datadir_path`, however not all places in the code use this util function. Using the util function everywhere makes it easier to review pull requests related to the datadir.
This commit replaces datadir path creation with the `datadir` member of `TestNode`, which itself uses `get_datadir_path`.
Tree-SHA512: c75707ab7149d732a6d56152a5813138a33459d3d07577b60b89f2a207c83b7663fac5f203593677c9892d1c23a5eba4bd45c5c4ababf040d720b437240fcddf
fae1374 qa: Allow for partial_match when checking init error (MarcoFalke)
5812273 [Tests] Require exact match in assert_start_raises_init_eror() (John Newbery)
0ec08a6 [Tests] Move assert_start_raises_init_error method to TestNode (John Newbery)
Pull request description:
Extracted from #12379, because the changes are important on their own.
This allows for exact testing, since the match can be specified with a strict regex. Internal details (such as exact formatting of the error message) can still be fuzzed away by regex wildcards.
Tree-SHA512: 605d2c9c42362a32d42321b066637577a026d0bb8cfc1c9f5737a4ca6503ffe85457a5122cea6e1101053ccc6c8aa1bbae3602e1fa7d2988bf7d5c275f412f66
fa23105 qa: Cache only chain and wallet for regtest datadir (MarcoFalke)
Pull request description:
mempool.dat should be empty and I don't see a need to copy it around when restoring from the cache.
Tree-SHA512: f11ab69732db4dee0e9a0900570464e49085532b0cebc963877057112a7b985c477da3d32eb2093daabac9ada9e73b7c49881681ec5efa6101919b0af76001cf
* Z is the zone designator for the zero UTC offset.
* T is the delimiter used to separate date and time.
This makes it clear for the end-user that the date/time logged is
specified in UTC and not in the local time zone.
Prevents OSX firewall allow-this-application-to-accept-inbound-connections
permission popups and is generally safer.
To prevent binding to 127.0.0.1, set self.bind_to_localhost_only = False.
* Pass `sys.executable` when calling a python script via the subprocess
module
* Don't remove the log file while it is still open and written to
* Properly use os.pathsep and os.path.sep when modifying the PATH
environment variable
* util-tests: Use os.path.join for Windows compatibility
Also warn if bitcoind is configured to use a relative -datadir path.
Specifying paths relative to the current working directory in a daemon process
can be dangerous, because files can fail to be located even if the
configuration doesn't change, but the daemon is started up differently.
Specifying a relative -datadir now adds a warning to the debug log. It would
not be backwards-compatible to forbid relative -datadir paths entirely, and it
could also be also inconvenient for command line testing.
Specifying a relative -walletdir now results in a startup error. But since the
-walletdir option is new in 0.16.0, there should be no compatibility issues.
Another reason not to use working directory paths for -walletdir specifically
is that the default -walletdir is a "wallets" subdirectory inside the datadir,
so it could be surprising that setting -walletdir manually would choose a
directory rooted in a completely different location.
test_framework accepts a new --usecli parameter. Running the test with
this parameter will cause all RPCs to be sent through bitcoin-cli rather
than directly over http. By default, individual test cases do not
support --usecli, and self.supports_cli must be set to True in the
set_test_params method.
We can make supports_cli default to True in future once we know which
tests will fail with use_cli.
fafa003 qa: Remove never used return value of sync_with_ping (MarcoFalke)
fa9de37 qa: Make tmpdir option an absolute path (MarcoFalke)
Pull request description:
This should fix issues with the multiwallet test and its symlinks
when the tmpdir is a relative path.
Rather than fixing os.symlink to work with paths relative to a
directory descriptor, which does not work on Windows, normalize
the path instead.
Tree-SHA512: 189690f3d065ea2f0f48e06775c86d513d0916c7c86312432e8e16df160e65539e288c2bd53d49a4180735fa940f6fcd52b506ccd7d9815651a9b1a69850dda6
This should fix issues with the multiwallet test and symlinks
when the tmpdir is a relative path.
Rather than fixing os.symlink to work with paths relative to a
directory descriptor, which does not work on Windows, normalize
the path instead.
