cln_plugin: multiple triggers for plugin shutdown

[justinmoon]

As far as I can tell, plugin authors need to handle 2 separate triggers to shut down our plugins:

1. We receive shutdown notification from lightnignd.
2. lightningd stopped and we didn't receive shutdown notification. This can be if we read from STDIN and receive 0 bytes.

Having to handle 2 different shutdown triggers complicate plugin code. Ideally plugin authors could pass a single callback to cln_plugin's plugin builder which would get run in either case. It also took me a long time to understand that I should be handling these 2 cases. Perhaps one explicit shutdown handler would make this clearer. If, for some reason, a plugin wanted to handle these differently, then an enum could be passed as an argument to this callback indicating which case needs handling.

[cdecker]

I think these are very much distinct: the shutdown notification is a courtesy heads up that gets delivered to the plugins that asked for it, while stdin closing is the definitive "die-now" signal that will always be delivered (unlike the shutdown notification which may be omitted). Since the shutdown notification is intended for plugin devs to implement any custom shutdown logic (release resources, commit changes, clean up temporary files, ...) we should not be processing it in cln-plugin ourselves, and instead allow library users to register it if needed.

The shutdown notification may also not be the place we actually want to kill the process, and rely instead on the more reliable stdin close event to exit ourselves.

[justinmoon]

Since the shutdown notification is intended for plugin devs to implement any custom shutdown logic (release resources, commit changes, clean up temporary files, ...)

What's the difference between cleaning up resources on shutdown vs stdin close? Seems like the latter is far more reliable. What's the benefit of also handling the former separately?

The only benefit of shutdown that I can think of is that core-lightning is still running so we could make RPC calls here, but couldn't once STDIN closes we can't.

Edit: looks like we can't do RPC with lightningd once we've received shutdown https://lightning.readthedocs.io/PLUGINS.html?highlight=shutdown#shutdown

[cdecker]

According to the docs the RPC is also no longer present:

In the shutdown case, plugins should not interact with lightnind except via (id-less) logging or notifications. New rpc calls will fail with error code -5 and (plugin’s) responses will be ignored. Because lightningd can crash or be killed, a plugin cannot rely on the shutdown notification always been send.

This means the two are pretty much identical, and shutdown is only a courtesy heads up that we're about to go down. This allows us to flush logs, or backups and other teardown operations. Internally it kicks off the memory leak detection to do one last pass, and ensure we don't have any leaks that are still reachable.

Being a courtesy notification, we cannot ensure it will always be delivered (if the lightningd process gets killed or we lose power it doesn't go through the shutdown routine), and that's also why it is purposefully limited, so devs don't rely too heavily on it, which may cause data corruption or an inconsistent state in their logic, if they assume a shutdown will always come, and then we get killed

[justinmoon]

Thanks for explaining. With #6041 we can just call plugin.shutdown() when we receive shutdown and then do all our cleanup in plugin.join(). So I'll close this issue now.