Helix pull upstream #10
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rThread twice Squash where I add interrupt
793667a Improve shutdown process (João Barbosa) Pull request description: Improve the shutdown time by not having to wait up to 2 seconds. Here is a comparison running `wallet.py` function tests before this PR: ``` 2017-08-08 03:25:20.881000 TestFramework (INFO): Initializing test directory /var/folders/1v/8_69hby54nj2k3n6fywt44x80000gn/T/testq_ramjjr 2017-08-08 03:25:23.853000 TestFramework (INFO): Mining blocks... 2017-08-08 03:25:24.132000 TestFramework (INFO): test getmemoryinfo 2017-08-08 03:25:24.559000 TestFramework (INFO): test gettxout 2017-08-08 03:25:59.858000 TestFramework (INFO): check -rescan 2017-08-08 03:26:07.735000 TestFramework (INFO): check -reindex 2017-08-08 03:26:15.751000 TestFramework (INFO): check -zapwallettxes=1 2017-08-08 03:26:24.105000 TestFramework (INFO): check -zapwallettxes=2 2017-08-08 03:26:36.694000 TestFramework (INFO): Stopping nodes 2017-08-08 03:26:43.599000 TestFramework (INFO): Cleaning up 2017-08-08 03:26:43.612000 TestFramework (INFO): Tests successful ``` After: ``` 2017-08-08 03:24:04.319000 TestFramework (INFO): Initializing test directory /var/folders/1v/8_69hby54nj2k3n6fywt44x80000gn/T/testoqeyi50_ 2017-08-08 03:24:07.035000 TestFramework (INFO): Mining blocks... 2017-08-08 03:24:07.317000 TestFramework (INFO): test getmemoryinfo 2017-08-08 03:24:07.763000 TestFramework (INFO): test gettxout 2017-08-08 03:24:25.715000 TestFramework (INFO): check -rescan 2017-08-08 03:24:27.792000 TestFramework (INFO): check -reindex 2017-08-08 03:24:29.797000 TestFramework (INFO): check -zapwallettxes=1 2017-08-08 03:24:32.207000 TestFramework (INFO): check -zapwallettxes=2 2017-08-08 03:24:36.812000 TestFramework (INFO): Stopping nodes 2017-08-08 03:24:37.915000 TestFramework (INFO): Cleaning up 2017-08-08 03:24:37.927000 TestFramework (INFO): Tests successful ``` This largely improves the time spent in Travis (under evaluation). Tree-SHA512: 023012fb3f8a380addf5995a4bf865862fed712cdd1a648d82a710e6566bc3bd34b6c49f9f06d6cc6bd81ca859da50d30d7f786c816e702549ab642e3476426f
28479f9 qa: Test bitcond shutdown (João Barbosa) 8d3f46e http: Remove timeout to exit event loop (João Barbosa) e98a9ee http: Remove unnecessary event_base_loopexit call (João Barbosa) 6b13580 http: Unlisten sockets after all workers quit (João Barbosa) 18e9685 http: Send "Connection: close" header if shutdown is requested (João Barbosa) 02e1e4e rpc: Add wait argument to stop (João Barbosa) Pull request description: Fixes bitcoin#11777. Reverts bitcoin#11006. Replaces bitcoin#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
6c10037 rpc: Fix data race (UB) in InterruptRPC() (practicalswift) Pull request description: Fix data race (UB) in `InterruptRPC()`. Before: ``` $ ./configure --with-sanitizers=thread $ make $ test/functional/test_runner.py feature_shutdown.py … SUMMARY: ThreadSanitizer: data race rpc/server.cpp:314 in InterruptRPC() … ALL | ✖ Failed | 2 s (accumulated) ``` After: ``` $ ./configure --with-sanitizers=thread $ make $ test/functional/test_runner.py feature_shutdown.py … ALL | ✓ Passed | 3 s (accumulated) ``` Tree-SHA512: b139ca1a0480258f8caa7730cabd7783a821d906630f51487750a6b15b7842675ed679747e1ff1bdade77d248807e9d77bae7bb88da54d1df84a179cd9b9b987
…raises_init_error 62c304e tests: Allow closed http server in assert_start_raises_init_error (Chun Kuan Lee) Pull request description: The rpc handler may be unregistered when http server haven't been closed yet. So it may be allowable to get -342 `non-JSON HTTP response with \'%i %s\' from server` (503 Service Unavailable) See https://ci.appveyor.com/project/DrahtBot/bitcoin/build/master.2001. It shows "Rejecting request while shutting down" between "RPC stopped" and "Stopped HTTP server" Tree-SHA512: e1f50ab9096cf23494ccc2850c01516c4a75f112c99108759157b22fce2011682a4b88216f206702f6a56e960182c00098053ad75f13aa0eafe27046adae63da
35c9b1e Fix data race (ale) Pull request description: Fix the data race in which a thread is trying to write `pcoinsTip.cacheSaplingAnchors`: `CWallet::BlockConnected -> pcoinsTip->GetSaplingAnchorAt(...)` At the same time another thread is trying to read `pcoinsTip.cacheSaplingAnchors`: `ActivateBestChain -> FlushStateToDisk -> pcoinsTip->DynamicMemoryUsage()`. The fix is trivial `CWallet::BlockConnected` was not locking the mutex `cs_main` that guards `pcoinsTip`. This data race was causing the functional test `wallet_basic.py` to randomly fail: To see this I created two test branches where I ran `wallet_basic.py` 50 times in parallel. One branch has this fix the other doesn't. The github action for the branch without this fix failed many times https://github.com/panleone/PIVX/actions/runs/8428460478 The github action for the branch with this fixed always passed instead https://github.com/panleone/PIVX/actions/runs/8428472901 ACKs for top commit: 35c9b1e Duddino: utACK 35c9b1e Liquid369: tACK 35c9b1e Fuzzbawls: ACK 35c9b1e Tree-SHA512: a5cea7f1b1478045f0e642c306f8f0427a10e92a13cc61e830bb214d4c5d1ba234cba1f628606ee02d0acc68c2719a981433f84fadbbd559af885e14e6353489
607ba81 test: add trivial validation invalid tests (ale) 2c87e78 test: add trivial validation tests (ale) 325a204 Verify match between type and tx version in GetValidatedTxPayload (ale) bf373bf Refactor special_tx_validation (ale) 78c7d26 Add SPECIAL_TX_TYPE member to payload classes (ale) 357f572 Add IsTriviallyValid for special payloads (ale) Pull request description: First 4 commits are a (almost) move only refactor: The self contained part of special transaction validation (i.e the part based only on specialtx info and not on full chain and masternode list) has been moved to functions `IsTriviallyValid`. Last 2 commits improve unit test coverage: I have added two files with a list of special txs which are either valid or invalid. Test consist in correctly deserializing the txs and verifying that valid/invalid ones are indeed considered valid/invalid. ACKs for top commit: 607ba81 Liquid369: tACK 607ba81 Duddino: utACK 607ba81 Fuzzbawls: ACK 607ba81 Tree-SHA512: 59a810d49c17943e2dfc10d95e7886922b42cfa2c91b6f8d77d64aa2a386fdebe137c11eb8f93188d79fcd0bae42aceeb2fc3d63af2a3abc1b2ac1cf103d31ea
471cae3 Do not sleep after staking and after sending pings (ale) Pull request description: `tiertwo_governance_sync_basic` is by far the slowest test that we have. Profiling showed that most of the time was spent in the function stake() which slept for a total of 2 seconds in each call. Those 2 seconds sleep were actually useless and have been removed. With this simple change the total running time of the test on my machine dropped from `15` minutes to only `6` minutes. To prove that indeed sleeping was useless I tried to run the same test in parallel 15 times, and it never failed, see here: https://github.com/panleone/PIVX/actions/runs/8420343441 ACKs for top commit: 471cae3 Duddino: utACK 471cae3 Liquid369: tACK 471cae3 Fuzzbawls: ACK 471cae3 Tree-SHA512: 6b00a18c7edd6dffaf9c5df3f53c8481c4312f7d8a63819010289c17b5ab7715aeafb901c8da885515fa59a093ab6e101c572972823ac2ca5e5c53139921f063
44116cf Separate Init from Start and Stop from Destroy functions for Chainlock handler (ale) 581eabe Add Interrupt step for signing_share thread and do NOT call StopWorkerThread twice (ale) 8e85ca2 promote chainLocksHandler to unique pointer (ale) 4f579eb promote quorumSigningManager to unique pointer (ale) 4820a63 promote quorumSigSharesManager to unique pointer (ale) Pull request description: Fix some issues in the LLMQ threads handling, that might be also causing some problems in some tests that randomly fail during the final `ShutDown()` phase. First 3 commits are just trivial refactor: Use unique pointers in place of normal pointers to be consistent with the style used for other llmq managers. Fourth commit: In `CSigSharesManager` the function `StopWorkerThread()` was being called twice. The wrong call was the one in the destructor which has been removed. The rest of the diff is basically a copy and paste from [net_masternodes.cpp](https://github.com/PIVX-Project/PIVX/blob/master/src/tiertwo/net_masternodes.cpp) (the variable `interruptNet` of that file plays the exact same role of the variable `interruptSigningShare` that I have added). This new version should be the right way to manage the thread Fifth commit: In the chainlock manager split the `Init` from the `Start` and the `Stop` from the `Destroy` phases, as we do for all other llmq managers. ACKs for top commit: 44116cf Duddino: utACK 44116cf Liquid369: tACK 44116cf Fuzzbawls: ACK 44116cf Tree-SHA512: 41a432781861753ce29904093fc6f8771d8745babd498526c5fa6890703de96078c6a242c42ebc292ddefa0c5c0488effb8f6f367771af229f066132a8004eba
05c30a8 Merge bitcoin#14413: tests: Allow closed rpc handler in assert_start_raises_init_error (MarcoFalke) cb65d0a Merge bitcoin#14993: rpc: Fix data race (UB) in InterruptRPC() (Wladimir J. van der Laan) cf6cb29 Merge bitcoin#14670: http: Fix HTTP server shutdown (Wladimir J. van der Laan) d19d9fa Call RPCNotifyBlockChange on RPC stopped (ale) af10755 Merge bitcoin#11006: Improve shutdown process (Wladimir J. van der Laan) Pull request description: The aim of this PR is fixing the functional test errors that we get sometimes during shutdown, for example the failure of this action https://github.com/PIVX-Project/PIVX/actions/runs/8429115630/job/23082877380 In a few words, on HTTP server shutdown, sometimes 2 seconds are not enough time to finish handling all the RPC requests. `2024-03-26T01:35:26Z HTTP event loop did not exit within allotted time, sending loopbreak` So after this point all existing RPC requests (in particular the `stop()` one) does not receive any answer... And this lead to functional tests failing at the end. For more info see the discussion in each backported PR. Backport bitcoin PRs bitcoin#11006 bitcoin#14670 bitcoin#14993 bitcoin#14413 ACKs for top commit: 05c30a8 Liquid369: tACK 05c30a8 Fuzzbawls: ACK 05c30a8 Tree-SHA512: 3fa2d7d9f11f851dbf3703a70e45a12905cb801156b32654f624487c386d76d4eebbc9d54418f4fbe0ac500afeed626636cda2dc942294ea991ff5cde47b07f4
Invoking configure from within cmake on macOS results in a broken c++ compiler check using system clang++. Work around this by running configure first outside of cmake.
Refactors the libgmp check during configure to use pkgconfig if available, and fallback to using autoconf's check header/check lib. Removes the m4 script as these checks can just be done directly in configure.ac.
Because we pass -qt-xcb to Qt, it will compile in a set of xcb helper libraries and extensions. So skip building all of the libxcb extensions when we build libcxb in depends. More info is available here: https://doc.qt.io/qt-5.9/linux-requirements.html
This also fixes passing --without-tools
By blanket passing --disable-dependency-tracking to all depends packages we end up with some warnings like: configure: WARNING: unrecognized options: --disable-dependency-tracking So instead, only pass it to packages that understand it.
xcb_proto's configure doesn't understand --disable-shared or --with-pic. All the package does it put a stack of xml files into a directory to be used by libxcb.
…ve builds de55dd9 CI: Run configure before cmake on macOS (Fuzzbawls) 5865b1e CI: Build BDB for native runners (Fuzzbawls) 2af6478 Contrib: Update install_db4.sh (Fuzzbawls) 8a21052 CI: Silence brew install warnings (Fuzzbawls) f2f456a CMake: Stop explicitly setting `BerkeleyDB_ROOT_DIR` (Fuzzbawls) ada1bae Build: Check if Homebrew BDB is actually installed (Fuzzbawls) Pull request description: This removes the dependence on system package managers to provide the BerkeleyDB library for native build jobs. Instead, use the supplied `install_db4.sh` script to compile and install BerkeleyDB into a system path (`/usr/local`). Rationale for this change is that Homebrew is going to remove BerkeleyDB 4 from it's package manager soon, so this is a proactive change to prevent CI build jobs from breaking when that happens. Homebrew based BerkeleyDB 4 support is still retained on our end for those that already have the package installed. Linux native CI build jobs are also switched over to this method of compiling/installing BerkeleyDB 4, rather than using the package manager's BerkeleyDB 5. ACKs for top commit: de55dd9 Duddino: ACK de55dd9 Liquid369: ACK de55dd9 Tree-SHA512: 337e058d836dd07f6e379e305c094c8c458f94bec044d126ffde7ef9e34e13fcbab0a09d1312922681c3964dd710b6d382b950dfa07a8bc5f714598447f33402
Looks like savannah deprecated the `git` subdomain in favor of `gitweb`, and is having server stability issues. Update the subdomain of the urls for newer config.sub and config.guess files to use a github gist instead.
151f6df Tools: Fix urls to new config.guess/sub (Fuzzbawls) Pull request description: Looks like savannah deprecated the `git` subdomain in favor of `gitweb`, and curl doesn't follow DNS redirects. Additionally, their servers seem to be having stability issues. Update the URLs to using a github gist instead. ACKs for top commit: 151f6df Duddino: utACK 151f6df Liquid369: tACK 151f6df Tree-SHA512: db6a5736cb930ea23b5a16dbec73807d673c70ea245e4f654c786c4e05d276fdaa82b5ce08111043d4b82699d17188a5babaae77d74ce9e47b1dc505758208cd
50f8701 Build: Remove no longer used old splash images (Fuzzbawls) 3ed1974 GUI: Update PIVX Shield Logos (Fuzzbawls) Pull request description: Updates the various images/icons that use the PIVX Shield logo to the newer design. Also removes the unused (since v5.0) old splash images. Note: the animated "processing" images will be a separate PR. ACKs for top commit: 50f8701 Duddino: ACK 50f8701 Liquid369: tACK 50f8701 Tree-SHA512: 205262381ce8fc4a546095beff5a25ad127862d822af2fae99d8246aee5e392bd1f134284bf24f7ef956233403fe7283c968f60b3be6dfe72b26c976b74961f6
7585906 [GitHub] Update to issue template forms (Fuzzbawls) Pull request description: Migrates from a single markdown based issue template to GitHub's newer YAML based form templates, which support multiple issue templates and extended features such as form validation and auto labeling.  Both of the new template forms can be viewed for review at the following links: - https://github.com/Fuzzbawls/PIVX/issues/new?template=01-bug-report.yml - https://github.com/Fuzzbawls/PIVX/issues/new?template=02-feature-request.yml The 'Blank issue' option allows for creating an issue without using a template, and the 'General Community Support' option redirects to `https://discord.pivx.org` ACKs for top commit: 7585906 Duddino: ACK 7585906 Liquid369: ACK 7585906 Tree-SHA512: e9dcd97221c4d9b2a34010c655d91199ac3c9ec3e621e04fb42baafe984ddca6131dfac8d1ae9003c76183e731d9683ae5d9006f90d51609e4c6b6df2dc7b5da
18b7faa [Build] Check for libgmp directly (Fuzzbawls) Pull request description: Refactors the libgmp check during configure to use pkgconfig if available, and fallback to using autoconf's check header/check lib. Removes the m4 script as these checks can just be done directly in configure.ac. ACKs for top commit: 18b7faa Duddino: ACK 18b7faa Liquid369: ACK 18b7faa Tree-SHA512: 89fb95dcb94768bfb44e45e6dc57e92572a300d5dfe30a79a44871ba5f7605320df9f3f0a2eda5ca456ffdb5e90756ea1495de57e7d97a0484e2c3a4d5bfb157
Ubuntu Bionic's libgmp-dev package doesn't include pkgconfig files, so we need to do some fallback checking during the configure process.
c8a7896 CI: Bump depends cache version (Fuzzbawls) 98c6066 depends: prune dbus from depends (fanquake) 1ed639e build: pass --enable-option-checking to applicable packages (fanquake) 0ff86a1 depends: don't configure xcb_proto (fanquake) 2059c26 build: don't always use --disable-dependency-tracking (Fuzzbawls) 9185414 depends: zeromq: disable draft classes and methods (fanquake) 6aecebb depends: xproto: configure flags cleanup (fanquake) e468f21 depends: qrencode: configure flags cleanup (fanquake) 2ca24cc depends: fontconfig: configure flags cleanup (fanquake) 6ca19a5 depends: libxcb: configure flags cleanup (fanquake) 22984ab depends: libXau: configure flags cleanup (fanquake) fb4fec1 build: disable libxcb extensions (fanquake) 678f261 depends: Purge libtool archives (Fuzzbawls) f4db397 depends: build secondary deps statically (Fuzzbawls) Pull request description: A collection of smaller upstream PRs aimed at cleaning up the depends package configurations. Included is: - bitcoin#15844 - bitcoin#16352 - bitcoin#16370 - bitcoin#16533 - bitcoin#16949 - bitcoin#17698 Package version updates are intentionally not included here, and will be done in separate PRs. ACKs for top commit: c8a7896 Duddino: ACK c8a7896 Liquid369: tACK c8a7896 Tree-SHA512: 2e415003ed06333793ecb3bd58ac3c12a8f8b6d1dfe7914c8e59b4c30c8fb58e4e9e28f3d2eba1491a46a611b2ba985273af9b46fc03015e51fcf37094f261ba
Detect and include the QT Image Formats plugins.
3e4c231 Build: Ubuntu Bionic gmp detection workaround (Fuzzbawls) Pull request description: Ubuntu Bionic's libgmp-dev package doesn't include pkgconfig files, so we need to do some fallback checking during the configure process. Bionic is still used for PPA and SnapCraft nightly builds, and we do still support Bionic in terms of glibc. ACKs for top commit: 3e4c231 Liquid369: ACK 3e4c231 Liquid369: ACK 3e4c231 Duddino: ACK 3e4c231 Tree-SHA512: 202bc3515a4b0ecf001e5b605f05e35e40fd1e9f08310c2ec0b6b37cee22a9a6d9f6a332384b9696bd8aa85406b9e34adb61d256c5cefa75b3190f969a2d40a6
4240f24 Doc: Add mention of webp (Fuzzbawls) 0461fda CI: Add Qt Image Formats Package (Fuzzbawls) efbe945 GUI: Replace loading gifs with webp animations (Fuzzbawls) 618ceb0 Build: Add Qt Image Formats detection (Fuzzbawls) ca99b81 Depends: Add Qt Image Formats (Fuzzbawls) Pull request description: The `.gif` format has many restrictions due to it's age, most notably alpha channels. This replaces the 'loading' animations with `.webp` files, that have much better fidelity. Thanks goes to RedByrd for providing the raw frame sequences. ACKs for top commit: 4240f24 Liquid369: ACK 4240f24 Duddino: ACK 4240f24 Tree-SHA512: 00fd30495bf53ee56a0f46fc81f9dc7767df7697a8c118b5580cc7dc7a5931b0ae02730078afe05f8065783c6cd26121150ca8f2adaa30c736f68ec5ded2d3d8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.