This function, which waits for all threads to exit, is no longer needed
now that threads are joined instead.
Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
zcash: cherry picked from commit f94665466ed50e868c98b1a1c708ad5767727bb6
zcash: https://github.com/bitcoin/bitcoin/pull/12366
This prevents a potential race condition if control flow ends up in
`ShutdownHTTPServer` before the thread gets to `queue->Run()`,
deleting the work queue while workers are still going to use it.
Meant to fix#12362.
Signed-off-by: Wladimir J. van der Laan <laanwj@gmail.com>
zcash: cherry picked from commit b1c2370dde9ade180c638e5d9a4797f085322b5b
zcash: https://github.com/bitcoin/bitcoin/pull/12366
The bug was introduced in 2.1.6-beta, versions before that don't need the
workaround.
zcash: currently, we build with libevent-2.1.12, so this fix is needed
zcash: cherry picked from commit 97932cd2689659addfbb58dc6148928b73af3bd0
zcash: https://github.com/bitcoin/bitcoin/pull/11593
A rare race condition may trigger while awaiting the body of a message, see
upsteam commit 5ff8eb26371c4dc56f384b2de35bea2d87814779 for details.
This may fix some reported rpc hangs/crashes.
zcash: currently, we build with libevent-2.1.12, so this fix is needed
zcash: cherry picked from commit 6b58360f9b64eb0b680a662fdfd590e47f115f44
zcash: https://github.com/bitcoin/bitcoin/pull/11593
This removes a "race" between Interrupt() and Run(), though it
should not effect any of our supported platforms.
zcash: cherry picked from commit 7b2d96b634f9fd283480caf3bece56138d0587e3
zcash: https://github.com/bitcoin/bitcoin/pull/9679
- The old patch is no longer necessary because of this upstream fix:
https://github.com/boostorg/build/pull/560
- Boost 1.72 removed a <deque> from an include, which exposed a missing
include in src/httpserver.cpp.
- Boost 1.73 moved function placeholders into the boost::placeholders
namespace.
- The new patch is a fix from just after Boost 1.74 was released, fixing
a warning that was missed.
along with mutex/condvar/bind/etc.
httpserver handles its own interruption, so there's no reason not to use std
threading.
While we're at it, may as well kill the BOOST_FOREACH's as well.
Use std::unique_ptr for handling work items.
This makes the code more RAII and, as mentioned in the comment, is what
I planned when I wrote the code in the first place.
Fix various thread assertion errors caused during shutdown.
Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6719
- bitcoin/bitcoin#6990
- bitcoin/bitcoin#8421
- Second commit only in this PR
- bitcoin/bitcoin#11006
I've cherry-picked the relevant commits, along with a note in each commit referring to the original Bitcoin commit ID (and the Zcash issue numbers where applicable). I've tested each issue with/without these patches applied.
Closes#2214, #2334, and #2554.
When using std::thread in place of boost::thread, letting the threads destruct
results in a std::terminate. According to the docs, the same thing should be
be happening in later boost versions:
http://www.boost.org/doc/libs/1_55_0/doc/html/thread/thread_management.html#thread.thread_management.thread.destructor
I'm unsure why this hasn't blown up already, but explicitly detaching can't
hurt.
Zcash: cherry-picked from commit d3773ca9aeb0d2f12dc0c5a0726778050c8cb455
This fixes#2554 (zcash-cli stop during getblocktemplate long poll
causes 'Assertion `!pthread_mutex_unlock(&m)' failed.')
This continues/fixes #6719.
`event_base_loopbreak` was not doing what I expected it to, at least in
libevent 2.0.21.
What I expected was that it sets a timeout, given that no other pending
events it would exit in N seconds. However, what it does was delay the
event loop exit with 10 seconds, even if nothing is pending.
Solve it in a different way: give the event loop thread time to exit
out of itself, and if it doesn't, send loopbreak.
This speeds up the RPC tests a lot, each exit incurred a 10 second
overhead, with this change there should be no shutdown overhead in the
common case and up to two seconds if the event loop is blocking.
As a bonus this breaks dependency on boost::thread_group, as the HTTP
server minds its own offspring.
Zcash: cherry-picked from commit a264c32e3321ae909ca59cb8ce8bf5d812dbc4e1
This makes sure that the event loop eventually terminates, even if an
event (like an open timeout, or a hanging connection) happens to be
holding it up.
Zcash: cherry-picked from commit ec908d5f7aa9ad7e3487018e06a24cb6449cc58b
Add a WaitExit() call to http's WorkQueue to make it delete the work
queue only when all worker threads stopped.
This fixes a problem that was reproducable by pressing Ctrl-C during
AppInit2:
```
/usr/include/boost/thread/pthread/condition_variable_fwd.hpp:81: boost::condition_variable::~condition_variable(): Assertion `!ret' failed.
/usr/include/boost/thread/pthread/mutex.hpp:108: boost::mutex::~mutex(): Assertion `!posix::pthread_mutex_destroy(&m)' failed.
```
I was assuming that `threadGroup->join_all();` would always have been
called when entering the Shutdown(). However this is not the case in
bitcoind's AppInit2-non-zero-exit case "was left out intentionally
here".
Zcash: cherry-picked from commit de9de2de361ab1355b976f17371d73e36fe3bf56
Fixes#2334 and #2214.
Shutting down the HTTP server currently breaks off all current requests.
This can create a race condition with RPC `stop` command, where the calling
process never receives confirmation.
This change removes the listening sockets on shutdown so that no new
requests can come in, but no longer breaks off requests in progress.
Meant to fix bitcoin/#6717.
Zcash: cherry-picked from commit 5e0c22135600fe36811da3b78216efc61ba765fb
Change the few occurrences of the deprecated `auto_ptr` to c++11 `unique_ptr`.
Silences the deprecation warnings.
Also add a missing `std::` for consistency.
The two timeouts for the server and client, are essentially different:
- In the case of the server it should be a lower value to avoid clients
clogging up connection slots
- In the case of the client it should be a high value to accomedate slow
responses from the server, for example for slow queries or when the
lock is contended
Split the options into `-rpcservertimeout` and `-rpcclienttimeout` with
respective defaults of 30 and 900.
Split StartHTTPServer into InitHTTPServer and StartHTTPServer to give
clients a window to register their handlers without race conditions.
Thanks @ajweiss for figuring this out.
Implement RPCTimerHandler for Qt RPC console, so that `walletpassphrase`
works with GUI and `-server=0`.
Also simplify HTTPEvent-related code by using boost::function directly.
Zcash: QT changes ommitted
- *Replace usage of boost::asio with [libevent2](http://libevent.org/)*.
boost::asio is not part of C++11, so unlike other boost there is no
forwards-compatibility reason to stick with it. Together with #4738 (convert
json_spirit to UniValue), this rids Bitcoin Core of the worst offenders with
regard to compile-time slowness.
- *Replace spit-and-duct-tape http server with evhttp*. Front-end http handling
is handled by libevent, a work queue (with configurable depth and parallelism)
is used to handle application requests.
- *Wrap HTTP request in C++ class*; this makes the application code mostly
HTTP-server-neutral
- *Refactor RPC to move all http-specific code to a separate file*.
Theoreticaly this can allow building without HTTP server but with another RPC
backend, e.g. Qt's debug console (currently not implemented) or future RPC
mechanisms people may want to use.
- *HTTP dispatch mechanism*; services (e.g., RPC, REST) register which URL
paths they want to handle.
By using a proven, high-performance asynchronous networking library (also used
by Tor) and HTTP server, problems such as #5674, #5655, #344 should be avoided.
What works? bitcoind, bitcoin-cli, bitcoin-qt. Unit tests and RPC/REST tests
pass. The aim for now is everything but SSL support.
Configuration options:
- `-rpcthreads`: repurposed as "number of work handler threads". Still
defaults to 4.
- `-rpcworkqueue`: maximum depth of work queue. When this is reached, new
requests will return a 500 Internal Error.
- `-rpctimeout`: inactivity time, in seconds, after which to disconnect a
client.
- `-debug=http`: low-level http activity logging
Wladimir J. van der Laan
Cory Fields
Matt Corallo
Jack Grigg
str4d
zancas
Daira Hopwood
Daniel Cousens
Homu
João Barbosa
Jason Davies
paveljanik
Gregory Maxwell