Use a semaphore or pipe for shutdown notification (skeees, laanwj)

[laanwj]

Instead of polling, use a semaphore (on WIN32) or a pipe (on POSIX) for shutdown notification. This avoids the need for a polling loop in bitcoind.

Extends #13186.

This does not change the polilng of ShutdownRequested() in the GUI. This is more complicated: on POSIX it would be possible to import the pipe[0] handle into the event loop, and call a handler when a character is detected, on Windows the StartShutdown() could make a callback that queues a qt signal at the GUI side. But I think that is better left for a later PR.

[skeees]

Oh cool - if you are going to split into platform dependent code - you can post to a pthreads semaphore from a unix signal handler.
Would also suggest that anyone waiting/reading from the semaphore/pipe immediately post/write back that way you could have multiple threads listening for shutdown as opposed to restricting it to only one as you do currently

[laanwj]

you can post to a pthreads semaphore from a unix signal handler.

I'm fairly sure none of the pthreads-api is signal safe, at least on all platforms.

Would also suggest that anyone waiting/reading from the semaphore/pipe immediately post/write back that way you could have multiple threads listening for shutdown as opposed to restricting it to only one as you do currently

We only need to wait for shutdown in one thread, so as I see it that additional complexity is not needed. If it happens to be needed in the future it can be added then.

Edit: Travis issue is interesting though: looks like there is a race at shutdown where the HTTP server doesn't get to send back the reply to stop(). I'd be surprised if it is new in this pull, though it probably comes to the foreground due to the instant shutdown notification. I remember previous discussion about this in #11006 (esp. #11006 (comment)).

[laanwj]

Added a commit that reverts 793667a (#11006) to hopefully make travis pass. Likely, this has to be conditional on libevent version to prevent adding a delay on shutdown in another place while fixing another.

Edit: that worked locally, but not on travis- weird.

[ken2812221]

Maybe that shutdown is too quick, add 50ms delay? It failed on my machine occasionally

[laanwj]

Maybe that shutdown is too quick, add 50ms delay? It failed on my machine occasionally

Adding a fixed delay is never a valid solution.

[ken2812221]

Once StopHTTPServer has been called in main thread ,the event loop in http thread exit. But http workqueue thread needs to use the event loop to call evhttp_send_reply, this could happen after the http thread event loop has already exit.

bitcoin/src/httpserver.cpp

Line 591 in be27048

HTTPEvent* ev = new HTTPEvent(eventBase, true, [req_copy, nStatus]{

[laanwj]

Closing this for now, I don't think it's that important, the current way works fine.