ipfsd not stopped when quitting app from OS

[hacdias]

Yesterday I installed a binary with the latest version from master, on Windows, and I set it to startup on login. Since IPFS Desktop doesn't shutdown correctly when Windows turns off, I always get this message:

I didn't found, yet, a way to listen for Windows shutdown but there must be a way because I know that many apps can even prevent Windows from turning off.

[olizilla]

Also the app does not start cleanly after choosing the fsck option on osx. The menubar appears, but the node is displayed as offline. Quiting the app brings up the fsck dialog again, and then same proess repeats.

[hacdias]

NOTE TO SELF: https://github.com/electron/electron/blob/master/docs/api/power-monitor.md

[hacdias]

So, there is no shutdown listener for Windows (see issue here: electron/electron#12810). Will use it for Linux and macOS. For Windows I'll try to find a workaround.

[hacdias]

macOS and Linux

We can play with shutdown event from PowerMonitor. More info here: https://github.com/electron/electron/blob/master/docs/api/power-monitor.md#event-shutdown-linux-macos

Windows

We can play with .NET code and make it listen for the shutdown event. There are libraries to include .NET code on Node.js such as Edge.

---

I can work on the Windows fix. Anyone willing to fix the macOS/Linux one? (not using any of the OSes ATM)

[olizilla]

i'll experiement with the osx shutdown logic.

[olizilla]

@hacdias before you get too deep into .NET code, have you tried listening to the before-quit event on windows?

[hacdias]

@olizilla I didn't. The docs say:

Note: On Windows, this event will not be emitted if the app is closed due to a shutdown/restart of the system or a user logout.

[hacdias]

May not need .NET. Just some C++.

[olizilla]

i think the problem here is we can't guarantee that the ipfd will ever be shutdown cleanly. We should what we can to shut it down, but there will be times when it's just not possible. Right now the ipfs daemon command will run just fine if there is an api file left over in the repo but ipfsd-ctl won't.

I think we have to go back to having desktop run some clean up logic if it finds the repo has evidence of an unclean shutdown. We've already got a couple of suggesitions to consider

• @Stebalien's proposal in feat: cleanup repository before starting up IPFS #722 (comment)
• @dryajov's propsoal in Trying to start non-disposable node fails on second attempt js-ipfsd-ctl#226 (comment)

[olizilla]

Of note running ipfs daemon also works fine if there is a repo.lock and a api file in the repo!

We should make ipfsd-ctl as robust as ipfs daemon is!

[olizilla]

@hacdias ipfs/js-ipfsd-ctl#315 - an issue for making ipfsd-ctl as robust as running ipfs daemon is. I think that would solve the core of our problem here. We'd try and shutdown gracefully where possible, but the app should still start next time if we fail.

cc @lidel

[hacdias]

@olizilla I think we should go for @Stebalien's proposal while js-ipfsd-ctl is not able to handle this situation. I can work on adding that logic in Desktop. What do you think?

[olizilla]

@hacdias yes, I think we should work around it for now. I think in the short term we have to deal with a either a repo.lock, an api file or both, so lets

• try to connect via ifpsd
• if it fails with an ECONNREFUSED
  • check if we've got an api file and no version file -> bail, as it's likely the user has a custom set up.
  • otherwise run ipfs repo fsk without prompting the user and try again.

I know we've been back and forth on this a lot, but this is basically what desktop 0.5 did, and no one has complained.

Theoretically we could try staring and stopping a daemon process directly to unblock things which seems to have the same effect as repo fsk in that it is robust to either an api file or a repo.lock being present and if allowed to shutdown gracefully, it will clean up after itself. This feels less agressive than running a repo fsk without asking, but I think it boils down to the exact same thing, but it's faster and less fiddly to run repo fsk than bring up a deamon just to let it clean up after itself.

I think we should continue to use ipfsd-ctl as we can help that module to bake this logic in rather than forcing all apps to duplicate it.

I think that not adding this work around is a blocker as it makes 0.6 feel way less stable than 0.5 as you only have to restart your machine with the app running to get an error.

cc @lidel

[Stebalien]

Theoretically we could try staring and stopping a daemon process directly to unblock things which seems to have the same effect as repo fsk in that it is robust to either an api file or a repo.lock being present and if allowed to shutdown gracefully, it will clean up after itself.

If it's not too much work, I believe this is the "most correct" approach as it will check if the repo is locked. repo fsck will remove the lockfile regardless.

[hacdias]

@Stebalien @olizilla I wouldn't mind to run the daemon directly but it would feel that we're duplicating the logic os ipfsd-ctl when we could just fix the library.

[olizilla]

Ipfsd-ctl deals with multiple configurations, so the change to add this feature to it needs some care. Please do create a PR to it you can see a clean way to add it. It may need an option to opt in to this behaviour or it might make more sense as a separate module that wraps ipfsd-ctl.

If that change proves more complicated, then it is fine for desktop to add a work atound; it's a more specific use-case, and we know we have access to a go-ipfs binary, so we can add step to start and then stop a daemon process directly, just to clean the repo, and then try running it via ipfsd-ctl again.

[hacdias]

@olizilla I think for now it's better if we just do it here and see how ipfs/js-ipfsd-ctl#315 goes. I can take a look at ipfsd-ctl afterwards. Although, keep in mind the logic I'm writing will only work for go-ipfs.