refactor: improve provider-keystore shutdown coordination

Refactor the provider shutdown coordination introduced in b30bc30c9
to make the dependency relationship more explicit and maintainable.

Changes:
- combine provider.Close() and keystore.Close() into a single
  ensureProviderClosesBeforeKeystore fx hook
- add detailed documentation explaining the race condition and
  why this shutdown order is critical
- make the code more self-documenting

The original fix in b30bc30c9 correctly solved the "keystore is closed"
shutdown errors. This refactor improves code clarity without changing
the functional behavior.
This commit is contained in:
Marcin Rataj 2025-11-07 17:20:38 +01:00
parent 7e49f06392
commit 5d2e6a751f

View File

@ -525,8 +525,41 @@ func SweepingProviderOpt(cfg *config.Config) fx.Option {
case <-ctx.Done():
return ctx.Err()
}
// Keystore data isn't purged, on close, but it will be overwritten
// when the node starts again.
// Keystore will be closed by ensureProviderClosesBeforeKeystore hook
// to guarantee provider closes before keystore.
return nil
},
})
})
// ensureProviderClosesBeforeKeystore manages the shutdown order between
// provider and keystore to prevent race conditions.
//
// The provider's worker goroutines may call keystore methods during their
// operation. If keystore closes while these operations are in-flight, we get
// "keystore is closed" errors. By closing the provider first, we ensure all
// worker goroutines exit and complete any pending keystore operations before
// the keystore itself closes.
type providerKeystoreShutdownInput struct {
fx.In
Provider DHTProvider
Keystore *keystore.ResettableKeystore
}
ensureProviderClosesBeforeKeystore := fx.Invoke(func(lc fx.Lifecycle, in providerKeystoreShutdownInput) {
// Skip for NoopProvider
if _, ok := in.Provider.(*NoopProvider); ok {
return
}
lc.Append(fx.Hook{
OnStop: func(ctx context.Context) error {
// Close provider first - waits for all worker goroutines to exit.
// This ensures no code can access keystore after this returns.
if err := in.Provider.Close(); err != nil {
logger.Errorw("error closing provider during shutdown", "error", err)
}
// Close keystore - safe now, provider is fully shut down
return in.Keystore.Close()
},
})
@ -550,25 +583,6 @@ func SweepingProviderOpt(cfg *config.Config) fx.Option {
}
}
type providerCloseInput struct {
fx.In
Provider DHTProvider
}
closeProvider := fx.Invoke(func(lc fx.Lifecycle, in providerCloseInput) {
// Skip for NoopProvider
if _, ok := in.Provider.(*NoopProvider); ok {
return
}
lc.Append(fx.Hook{
OnStop: func(ctx context.Context) error {
// Close the provider. This must happen before keystore is closed to
// prevent provider from writing to keystore after close.
return in.Provider.Close()
},
})
})
type alertInput struct {
fx.In
Provider DHTProvider
@ -671,7 +685,7 @@ See docs: https://github.com/ipfs/kubo/blob/master/docs/config.md#providedhtmaxw
return fx.Options(
sweepingReprovider,
initKeystore,
closeProvider, // Must be after initKeystore to ensure provider closes before keystore
ensureProviderClosesBeforeKeystore,
reprovideAlert,
)
}