fix: avoid duplicate daemon start during startup - #3210
Conversation
|
Windows test failure seems unrelated to my change, as it fails to find an ipfs binary for whatever reason as then just runs in a timeout. Test seems to be overall flaky. |
Only setupDaemon depends on the normalized flags, so start the splash window first and gate the daemon alone rather than the whole wave. The flag initializers run synchronously today, so this costs nothing now, but it keeps a future await in them from blocking the splash window alongside the daemon.
createSplashScreen left the splashScreen context prop unset when the window could not be built or the page failed to load. Nothing else resolves that deferred, so setupWebUI blocked on it forever and the startup Promise.all never settled: no web ui, no error dialog, no way to tell what went wrong. Publish the prop either way, null on failure, and let the consumers carry on without a splash.
startIpfs and stopIpfs decide what to do by reading ipfsd, which is only accurate between transitions. During a start it is still null, so a second start passed the guard and spawned a Kubo that raced the first for the repo lock; during a stop it is still set, so a second stop called stop() on a daemon already stopping. Queue every transition so the guards mean what they say, and wrap restart as a unit so nothing slips between its stop and its start. A config change arriving mid-start now applies once the start finishes instead of racing it.
lidel
left a comment
There was a problem hiding this comment.
Thank you, LGTM.
Pushed three commits on top:
-
02435fb fix: keep splash first while gating daemon start
The barrier gated the whole wave, so the splash window was not built until all three initializers resolved. -
489427b fix: do not stall startup when the splash fails
Semi-related:createSplashScreenleft thesplashScreencontext prop unset on failure, and nothing else resolves that deferred, sosetupWebUIblocked on it forever. -
1af7d82 fix: run daemon transitions one at a time
startandstopreadipfsd, which is only accurate between transitions, so a tray toggle or an online-status flip landing mid-restart could still spawn a second Kubo.
During packaging I noticed the following issue:
setupDaemon()currently starts Kubo and installs theIPFS_CONFIG_CHANGEDrestart listener in parallel withsetupAutoGc(),setupPubsub()andsetupNamesysPubsub().Those initializers can normalize the stored daemon flags. For example,
setupAutoGc()adds--enable-gcwhen it is missing and emitsIPFS_CONFIG_CHANGED.If that happens while the initial daemon start is still running, the restart handler starts a second Kubo against the same repo. One of them then fails with:
Error: lock .../repo.lock: someone else has the lockRun the three daemon flag initializers before
setupDaemon(). They can still update the stored flags, but there is no restart listener yet and the daemon starts once afterwards with the final flags.I've added a regression test which blocks all three initializers and verifies that
setupDaemon()does not run until they finished.