lightningd: shutdown plugins in two steps, db_write plugins as last

[SimonVrouwe]

Addresses issue #5859 to keep db_write-plugins alive when lightningd is shutting down, until after the database is closed.

This revives an old idea (from #4790) to call shutdown_plugins twice:

• In the first call it sends "shutdown" notifications (jsonrpc still exist) and waits for all non-db_write plugins to terminate.
• In the second call, after database is closed, remaining db_write plugins can safely self-terminate when they see an EOF event at their stdin.

One test_htlc_accepted_hook_shutdown was added (currently skipped) to demonstrate a hang when L1 tries to pay an invoice while L2 is shutting down.

Changed this into pull request after squashing, the unsquashed draft can be found here

TODO:

• update documentation and add changelog

[SimonVrouwe]

Weirdly the one failing (canceled) test log shows:

 tests/test_plugin.py::test_htlc_accepted_hook_shutdown 
Error: The operation was canceled.

which is exactly the test added in 2cb93d4 to demonstrate the hang, but the test was disabled!

@unittest.skipIf(True, "l1.rpc.pay hangs forever when called during l2's shutdown")
def test_htlc_accepted_hook_shutdown(node_factory, executor):

Before a rebasing/squash I will put some effort in understanding the new shutdown logic introduced in e7fe59b 375215a and its relation to the bookkeeper plugin and (maybe) the hang.

edit: if anything indicative, test_htlc_accepted_hook_shutdown (when enabled) starts hanging right after e7fe59b375215a, passes before it.

[m-schmoock]

@endothermicdev should we consider this for v23.02 milestone, as currently the usage of the widely used and important backup plugin is affected?

[SimonVrouwe]

Cleanup, rewording and added two asserts. Removed unnecessary sleeps and delays.

I removed test_htlc_accepted_hook_shutdown as the issue with a hang in L1.pay while L2 is shutting down, looks more an issue of the pay command then of the shutdown sequence as sendpay + waitsendpay doesn't hang. It also has nothing to do with htlc_hook, see old draft branch.

Note that a 1 millisecond timer is used when the io_loop only needs to send notifications, which is assumed to be long enough to make it to:

lightning/ccan/ccan/io/poll.c

Line 453 in f1373fd

io_ready(c, events);

and that all notifications are written out in one cycle.

Then rebased and squashed.

[SimonVrouwe]

I got stuck translating into human language what is happening with (other) hooks during shutdown, as it doesn't seem defined very well, see #5859. I will close this while waiting for answers.