rpc: stop accepting HTTP RPC before shutdown #35184

Refs #34262

getblocktemplate asks Bitcoin Core to build a candidate block for mining.

The problem is that an RPC request can be accepted before shutdown starts, but sit in the HTTP RPC queue and only begin running after shutdown has already started.

During shutdown, Bitcoin Core wakes up mining code that is waiting. So if the queued request is getblocktemplate, it can continue into block template creation after RPC shutdown has started. That can hit the existing CHECK_NONFATAL(block_template) check and return a less helpful internal RPC error instead of the normal shutdown RPC error.

This PR handles that queued-shutdown path by checking for RPC interruption once, early in JSONRPCExec(), before command execution starts. It also interrupts RPC earlier in shutdown, before waking mining waiters.

This keeps the existing CHECK_NONFATAL in getblocktemplate. The check still catches unexpected internal bugs, while this shutdown case should be handled before the command starts running.

To test this, the new regression test fills both RPC worker threads with longpolling getblocktemplate calls, then queues one more getblocktemplate request behind them. It then shuts down the node and checks that the queued request returns the normal shutdown RPC error.

Testing:

• cmake --build build --target test_bitcoin -j 8
• build/bin/test_bitcoin -t rpc_tests
• test/functional/mining_getblocktemplate_longpoll.py --configfile build/test/config.ini

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Did you use an LLM to write this? Can you rewrite the pull request description in your own words? I find your current description hard to follow. The testing instructions also don't really tell me anything.

Did you use an LLM to write this? Can you rewrite the pull request description in your own words? I find your current description hard to follow. The testing instructions also don't really tell me anything.

hey!

Yup, used an LLM. Just fixed it up.

Let me know if that makes more sense.

That can hit the existing CHECK_NONFATAL(block_template) assertion.

CHECK_NONFATAL throws an exception, which gets caught by the RPC handler and returned to the user. The reported error message might not be the best, and could be improved, but it shouldn't abort the program.

Also, the scenario you are describing should still be possible even with your modifications. It should just require a sleep time after the interruption check you added.

That said, a single RpcInterruptionPoint() call early in JSONRPCExec seems good to have regardless.

Thanks for following up @furszy

Did you need me to update the PR description given your callout, or is anything else needed from me on this one?

Did you need me to update the PR description given your callout, or is anything else needed from me on this one?

Having accurate information in the PR description helps for sure.

If you prefer to keep the introduced RpcRequestInterruptionPoint() instead of using RpcInterruptionPoint(), it would be good to explain the reasoning for it. Also, the interruption function should be called only once early in the function, not in both conditional paths. And it would also be nice to know why the InterruptRPC() call was moved.

Thanks, updated.

• I dropped the new RpcRequestInterruptionPoint() helper and now call the existing RpcInterruptionPoint() once early in JSONRPCExec(), before dispatching the command.
• I also updated the PR description to narrow the claim to this queued-shutdown path, changed Fixes [#34262](/bitcoin-bitcoin/34262/) to Refs [#34262](/bitcoin-bitcoin/34262/), and clarified that CHECK_NONFATAL returns through the RPC error path rather than aborting the process.
• I kept the InterruptRPC() move and added the reasoning to the description: RPC should be marked interrupted before the mining longpoll waiters are woken.

I feel like we are just prompting the author's LLM here. That is probably not the best way to spend review time. The testing section in the description still says nothing about the actual issue. If you want to contribute to Bitcoin Core, please start with reviewing some pull requests yourself.