diff --git a/go/kbfs/libkbfs/favorites.go b/go/kbfs/libkbfs/favorites.go index 240a01f33289..8d98341c02b8 100644 --- a/go/kbfs/libkbfs/favorites.go +++ b/go/kbfs/libkbfs/favorites.go @@ -70,6 +70,7 @@ type favReq struct { favs chan<- []favorites.Folder favsAll chan<- keybase1.FavoritesResult homeTLFInfo *homeTLFInfo + loadDisk bool // For asynchronous refreshes, pass in the Favorites from the server here favResult *keybase1.FavoritesResult @@ -130,6 +131,7 @@ type Favorites struct { shutdownChan chan struct{} muShutdown sync.RWMutex shutdown bool + loopOnce sync.Once } func newFavoritesWithChan(config Config, reqChan chan *favReq) *Favorites { @@ -157,10 +159,19 @@ func newFavoritesWithChan(config Config, reqChan chan *favReq) *Favorites { bufferedInterval: defaultFavoritesBufferedReqInterval, shutdownChan: make(chan struct{}), } - return f } +func (f *Favorites) startLoop() { + if f.disabled { + return + } + f.loopOnce.Do(func() { + f.loopWG.Add(1) + go f.loop() + }) +} + // NewFavorites constructs a new Favorites instance. func NewFavorites(config Config) *Favorites { return newFavoritesWithChan(config, make(chan *favReq, 100)) @@ -290,27 +301,25 @@ func (f *Favorites) writeCacheToDisk(ctx context.Context) error { // InitForTest starts the Favorites cache's internal processing loop without // loading cached favorites from disk. func (f *Favorites) InitForTest() { - if f.disabled { - return - } - go f.loop() + f.startLoop() } -// Initialize loads the favorites cache from disk and starts listening for -// requests asynchronously. +// Initialize starts the processing loop and loads the favorites cache from +// disk. Other methods start the loop on first use so notifications that +// arrive before this can still be processed. func (f *Favorites) Initialize(ctx context.Context) { if f.disabled { return } - // load cache from disk - err := f.readCacheFromDisk(ctx) - if err != nil { + req := &favReq{ + ctx: ctx, + loadDisk: true, + done: make(chan struct{}), + } + if err := f.sendReq(ctx, req); err != nil { f.log.CWarningf( ctx, "Failed to read cached favorites from disk: %v", err) } - - // launch background loop - go f.loop() } func (f *Favorites) closeReq(req *favReq, err error) { @@ -441,6 +450,10 @@ func (f *Favorites) handleReq(req *favReq) (err error) { } }() + if req.loadDisk { + return f.readCacheFromDisk(req.ctx) + } + if req.refresh && !req.buffered { <-f.refreshWaiting } @@ -674,7 +687,6 @@ func (f *Favorites) handleReq(req *favReq) (err error) { } func (f *Favorites) loop() { - f.loopWG.Add(1) defer f.loopWG.Done() bufferedTicker := time.NewTicker(f.bufferedInterval) defer bufferedTicker.Stop() @@ -759,13 +771,20 @@ func (f *Favorites) waitOnReq(ctx context.Context, } } -func (f *Favorites) sendReq(ctx context.Context, req *favReq) error { +func (f *Favorites) enqueue(ctx context.Context, req *favReq) error { + f.startLoop() f.wg.Add(1) select { case f.reqChan <- req: + return nil case <-ctx.Done(): f.wg.Done() - err := ctx.Err() + return ctx.Err() + } +} + +func (f *Favorites) sendReq(ctx context.Context, req *favReq) error { + if err := f.enqueue(ctx, req); err != nil { f.closeReq(req, err) return err } @@ -835,14 +854,8 @@ func (f *Favorites) AddAsync(ctx context.Context, fav favorites.ToAdd) { // if the original context is canceled. req, doSend := f.startOrJoinAddReq(context.Background(), fav) if doSend { - f.wg.Add(1) - select { - case f.reqChan <- req: - case <-ctx.Done(): - f.wg.Done() - err := ctx.Err() + if err := f.enqueue(ctx, req); err != nil { f.closeReq(req, err) - return } } } @@ -914,36 +927,27 @@ func (f *Favorites) RefreshCache(ctx context.Context, mode FavoritesRefreshMode) done: make(chan struct{}), ctx: context.Background(), } - f.wg.Add(1) if mode == FavoritesRefreshModeBlocking { favResult, err := f.config.KBPKI().FavoriteList(ctx) if err != nil { f.log.CDebugf(ctx, "Failed to refresh cached Favorites: %+v", err) - // Because the request will not make it to the main processing - // loop, mark it as done and clear the refresh channel here. - f.wg.Done() <-f.refreshWaiting return } req.favResult = &favResult } - select { - case f.reqChan <- req: - go func() { - <-req.done - if req.err != nil { - f.log.CDebugf(ctx, "Failed to refresh cached Favorites ("+ - "error in main loop): %+v", req.err) - } - }() - case <-ctx.Done(): - // Because the request will not make it to the main processing - // loop, mark it as done and clear the refresh channel here. - f.wg.Done() + if err := f.enqueue(ctx, req); err != nil { <-f.refreshWaiting return } + go func() { + <-req.done + if req.err != nil { + f.log.CDebugf(ctx, "Failed to refresh cached Favorites ("+ + "error in main loop): %+v", req.err) + } + }() } // RefreshCacheWhenMTimeChanged refreshes the cached favorites, but @@ -960,6 +964,7 @@ func (f *Favorites) RefreshCacheWhenMTimeChanged( if f.disabled || f.shutdown { return } + f.startLoop() req := &favReq{ refresh: true, @@ -1003,13 +1008,7 @@ func (f *Favorites) ClearCache(ctx context.Context) { done: make(chan struct{}), ctx: context.Background(), } - f.wg.Add(1) - select { - case f.reqChan <- req: - case <-ctx.Done(): - f.wg.Done() - return - } + _ = f.enqueue(ctx, req) } // GetFolderWithFavFlags returns the a FolderWithFavFlags for give folder, if found. @@ -1086,13 +1085,7 @@ func (f *Favorites) setHomeTLFInfo(ctx context.Context, info homeTLFInfo) { done: make(chan struct{}), ctx: context.Background(), } - f.wg.Add(1) - select { - case f.reqChan <- req: - case <-ctx.Done(): - f.wg.Done() - return - } + _ = f.enqueue(ctx, req) } // GetAll returns the logged-in user's list of favorite, new, and ignored TLFs. diff --git a/go/kbfs/libkbfs/init.go b/go/kbfs/libkbfs/init.go index 3457d1b217d8..a4e7d1310fb2 100644 --- a/go/kbfs/libkbfs/init.go +++ b/go/kbfs/libkbfs/init.go @@ -797,40 +797,34 @@ func doInit( kbfsLog := config.MakeLogger("") - // Initialize Keybase service connection. This needs to happen before - // KBPKI client. - if keybaseServiceCn == nil { - keybaseServiceCn = keybaseDaemon{} - } - service, err := keybaseServiceCn.NewKeybaseService( - config, params, kbCtx, kbfsLog) - if err != nil { - return nil, fmt.Errorf("problem creating service: %s", err) - } - if registry := config.MetricsRegistry(); registry != nil { - service = NewKeybaseServiceMeasured(service, registry) - } - config.SetKeybaseService(service) - - // Initialize KBPKI client (needed for KBFSOps, MD Server, and Chat). + // Initialize KBPKI client (needed for KBFSOps, MD Server, and Chat). It + // reaches the service through config, so it doesn't need it yet. k := NewKBPKIClient(config, kbfsLog) config.SetKBPKI(k) - // Initialize Chat client (for file edit notifications). - chat, err := keybaseServiceCn.NewChat(config, params, kbCtx, kbfsLog) - if err != nil { - return nil, fmt.Errorf("problem creating chat: %s", err) - } - config.SetChat(chat) - + // Set up KBFSOps and MDOps before the service connection. Creating the + // connection registers the KBFS handlers, and the service can call them + // right away. None of these use the service until they're called. initDoneCh := make(chan struct{}) kbfsOps := NewKBFSOpsStandard(kbCtx, config, initDoneCh) - defer close(initDoneCh) + // Handlers on the service connection wait for init (see + // waitForKBFSReady), so tell them how it ended. + initSucceeded := false + defer func() { + if initSucceeded { + close(initDoneCh) + } else { + kbfsOps.initFailed() + } + }() config.SetKBFSOps(kbfsOps) config.SetNotifier(kbfsOps) config.SetKeyManager(NewKeyManagerStandard(config)) config.SetMDOps(NewMDOpsStandard(config)) + // Also before the service connection: a login it delivers can create the + // disk block cache, which expects the disk limiter. The limiter only reads + // local config. config.SetDiskBlockCacheFraction(getCacheFrac( ctx, kbCtx, params.DiskBlockCacheFraction, defaultDiskBlockCacheFraction, configBlockCacheDiskMaxFracStr, log)) @@ -843,6 +837,27 @@ func doInit( return nil, err } + // Initialize Keybase service connection. + if keybaseServiceCn == nil { + keybaseServiceCn = keybaseDaemon{} + } + service, err := keybaseServiceCn.NewKeybaseService( + config, params, kbCtx, kbfsLog) + if err != nil { + return nil, fmt.Errorf("problem creating service: %s", err) + } + if registry := config.MetricsRegistry(); registry != nil { + service = NewKeybaseServiceMeasured(service, registry) + } + config.SetKeybaseService(service) + + // Initialize Chat client (for file edit notifications). + chat, err := keybaseServiceCn.NewChat(config, params, kbCtx, kbfsLog) + if err != nil { + return nil, fmt.Errorf("problem creating chat: %s", err) + } + config.SetChat(chat) + kbfsOps.favs.Initialize(ctx) config.SetReporter(NewReporterKBPKI(config, 10, 1000)) @@ -969,6 +984,19 @@ func doInit( } } + if params.BGFlushDirOpBatchSize < 1 { + return nil, fmt.Errorf( + "Illegal sync batch size: %d", params.BGFlushDirOpBatchSize) + } + log.CDebugf(ctx, "Enabling a dir op batch size of %d", + params.BGFlushDirOpBatchSize) + config.SetBGFlushDirOpBatchSize(params.BGFlushDirOpBatchSize) + + // Requests on the service connection have what they need from here on. + // Don't hold them for journaling, which can take a while. Nothing after + // this point may fail init, since released requests can't be recalled. + kbfsOps.initReady() + ctx60s, cancel := context.WithTimeout(ctx, 60*time.Second) defer cancel() // TODO: Don't turn on journaling if either -bserver or @@ -983,18 +1011,11 @@ func doInit( log.CDebugf(ctx, "Journaling enabled") } - if params.BGFlushDirOpBatchSize < 1 { - return nil, fmt.Errorf( - "Illegal sync batch size: %d", params.BGFlushDirOpBatchSize) - } - log.CDebugf(ctx, "Enabling a dir op batch size of %d", - params.BGFlushDirOpBatchSize) - config.SetBGFlushDirOpBatchSize(params.BGFlushDirOpBatchSize) - if config.Mode().OldStorageRootCleaningEnabled() { go cleanOldTempStorageRoots(config) } + initSucceeded = true return config, nil } diff --git a/go/kbfs/libkbfs/init_test.go b/go/kbfs/libkbfs/init_test.go new file mode 100644 index 000000000000..95486431d326 --- /dev/null +++ b/go/kbfs/libkbfs/init_test.go @@ -0,0 +1,206 @@ +// Copyright 2026 Keybase Inc. All rights reserved. +// Use of this source code is governed by a BSD +// license that can be found in the LICENSE file. + +package libkbfs + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "github.com/keybase/client/go/kbconst" + "github.com/keybase/client/go/kbfs/env" + "github.com/keybase/client/go/kbfs/idutil" + kbname "github.com/keybase/client/go/kbun" + "github.com/keybase/client/go/libkb" + "github.com/keybase/client/go/logger" + "github.com/keybase/client/go/protocol/keybase1" + "github.com/keybase/go-framed-msgpack-rpc/rpc" + "github.com/stretchr/testify/require" +) + +// initTestContext is the minimal Context doInit and NewKeybaseDaemonRPC need +// in tests. It has no sockets, so connections just keep failing to dial. +type initTestContext struct { + env.EmptyAppStateUpdater + env *libkb.Env + dataDir string +} + +var _ Context = (*initTestContext)(nil) + +func newInitTestContext(t *testing.T) *initTestContext { + return &initTestContext{ + env: libkb.NewEnv(nil, nil, func() logger.Logger { + return logger.NewNull() + }), + dataDir: t.TempDir(), + } +} + +var errNoSocket = errors.New("no socket in test") + +func (c *initTestContext) GetRunMode() kbconst.RunMode { return kbconst.DevelRunMode } +func (c *initTestContext) GetLogDir() string { return c.dataDir } +func (c *initTestContext) GetDataDir() string { return c.dataDir } +func (c *initTestContext) GetEnv() *libkb.Env { return c.env } + +func (c *initTestContext) GetMountDir() (string, error) { + return "", errors.New("no mount dir in test") +} + +func (c *initTestContext) ConfigureSocketInfo() error { return nil } +func (c *initTestContext) CheckService() error { return nil } + +func (c *initTestContext) GetSocket(bool) (net.Conn, rpc.Transporter, bool, error) { + return nil, nil, false, errNoSocket +} + +func (c *initTestContext) NewRPCLogFactory() rpc.LogFactory { return nil } + +func (c *initTestContext) NewNetworkInstrumenter( + keybase1.NetworkSource, +) rpc.NetworkInstrumenterStorage { + return nil +} + +func (c *initTestContext) GetKBFSSocket(bool) (net.Conn, rpc.Transporter, bool, error) { + return nil, nil, false, errNoSocket +} + +func (c *initTestContext) BindToKBFSSocket() (net.Listener, error) { + return nil, errNoSocket +} + +func (c *initTestContext) GetVDebugSetting() string { return "" } +func (c *initTestContext) GetPerfLog() logger.Logger { return logger.NewNull() } + +// newGatedTestProtocol returns a one-method protocol, standing in for +// SimpleFS/git/fs, that counts the calls that reach its handler. +func newGatedTestProtocol(calls *int) rpc.Protocol { + return rpc.Protocol{ + Name: "gatedTest", + Methods: map[string]rpc.ServeHandlerDescription{ + "method": {Handler: func(context.Context, any) (any, error) { + *calls++ + return nil, nil + }}, + }, + } +} + +var errInitTestCrypto = errors.New("crypto unavailable in test") + +// initOrderCn stands in for the service. Both while its connection is being +// built and once init has set it, it calls into KBFS the way the live service +// can while init is still running. NewCrypto then fails init. +type initOrderCn struct { + t *testing.T + config Config + daemon *KeybaseDaemonRPC + gated func(context.Context, any) (any, error) + calls int +} + +func (c *initOrderCn) NewKeybaseService( + config Config, _ InitParams, _ Context, log logger.Logger, +) (KeybaseService, error) { + c.config = config + name := kbname.NormalizedUsername("fake username") + c.daemon = newKeybaseDaemonRPC(config, nil, log) + config.SetKeybaseService(c.daemon) + c.daemon.fillClients(&fakeKeybaseClient{session: idutil.SessionInfo{ + Name: name, + UID: keybase1.MakeTestUID(1), + CryptPublicKey: idutil.MakeLocalUserCryptPublicKeyOrBust(name), + VerifyingKey: idutil.MakeLocalUserVerifyingKeyOrBust(name), + }}) + gated := gateOnKBFSReady(config, []rpc.Protocol{newGatedTestProtocol(&c.calls)}) + c.gated = gated[0].Methods["method"].Handler + c.callIntoKBFS() + return c.daemon, nil +} + +func (c *initOrderCn) NewChat( + config Config, _ InitParams, _ Context, _ logger.Logger, +) (Chat, error) { + c.callIntoKBFS() + return newChatLocal(config), nil +} + +func (c *initOrderCn) NewCrypto( + Config, InitParams, Context, logger.Logger, +) (Crypto, error) { + return nil, errInitTestCrypto +} + +func (c *initOrderCn) callIntoKBFS() { + t := c.t + ctx := context.Background() + for _, r := range []keybase1.Reachable{ + keybase1.Reachable_YES, keybase1.Reachable_NO, + } { + require.NoError(t, c.daemon.ReachabilityChanged( + ctx, keybase1.Reachability{Reachable: r})) + } + require.NoError(t, c.daemon.FavoritesChanged(ctx, keybase1.UID(""))) + require.NoError(t, c.daemon.TeamChangedByID(ctx, keybase1.TeamChangedByIDArg{ + Changes: keybase1.TeamChangeSet{Renamed: true}, + })) + require.NoError(t, c.daemon.TeamAbandoned(ctx, keybase1.TeamID(""))) + session, err := c.daemon.CurrentSession(ctx, 0) + require.NoError(t, err) + require.NoError(t, c.daemon.PaperKeyCached( + ctx, keybase1.PaperKeyCachedArg{Uid: session.UID})) + require.NoError(t, c.daemon.LoggedOut(ctx)) + + // Until init is ready, requests get an error or wait. + _, err = c.daemon.GetTLFCryptKeys(ctx, keybase1.TLFQuery{TlfName: "testuser"}) + require.Equal(t, errKBFSNotInitialized{}, err) + err = c.daemon.StartMigration(ctx, keybase1.Folder{Name: "testuser"}) + require.Equal(t, errKBFSNotInitialized{}, err) + waitCtx, cancel := context.WithTimeout(ctx, 20*time.Millisecond) + defer cancel() + _, err = c.gated(waitCtx, nil) + require.ErrorIs(t, err, context.DeadlineExceeded) +} + +// The service can call into KBFS as soon as its connection is up, which is +// before init finishes. KBFSOps and MDOps must already be set by then, and +// once init fails, requests must fail instead of waiting. +func TestInitSetsUpKBFSBeforeService(t *testing.T) { + cn := &initOrderCn{t: t} + kbCtx := newInitTestContext(t) + initReturned := false + // Registered after TempDir, so it runs first and closes the favorites + // db before the directory is removed. Skipped if doInit stopped partway + // (a failed assertion), since the favorites Shutdown can then block. + t.Cleanup(func() { + if !initReturned { + return + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + require.NoError(t, cn.config.KBFSOps().Shutdown(ctx)) + }) + params := DefaultInitParams(kbCtx) + params.StorageRoot = kbCtx.dataDir + params.DiskCacheMode = DiskCacheModeLocal + params.EnableJournal = false + + _, err := doInit( + context.Background(), kbCtx, params, cn, logger.NewTestLogger(t), "test") + initReturned = true + require.ErrorContains(t, err, errInitTestCrypto.Error()) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _, err = cn.gated(ctx, nil) + require.Equal(t, errKBFSNotInitialized{}, err) + _, err = cn.daemon.GetTLFCryptKeys(ctx, keybase1.TLFQuery{TlfName: "testuser"}) + require.Equal(t, errKBFSNotInitialized{}, err) + require.Zero(t, cn.calls) +} diff --git a/go/kbfs/libkbfs/kbfs_ops.go b/go/kbfs/libkbfs/kbfs_ops.go index 549f70d2cd99..7e2d18f97f4f 100644 --- a/go/kbfs/libkbfs/kbfs_ops.go +++ b/go/kbfs/libkbfs/kbfs_ops.go @@ -51,6 +51,12 @@ type KBFSOpsStandard struct { // watcher. reIdentifyControlChan chan chan<- struct{} initDoneCh <-chan struct{} + // Closed once, under initMu: ready if requests can run, failed if + // init failed before that. initFailed is a no-op after ready. + initReadyCh chan struct{} + initFailedCh chan struct{} + initMu sync.Mutex + initSignaled bool favs *Favorites @@ -70,7 +76,10 @@ type KBFSOpsStandard struct { initSyncCancel context.CancelFunc } -var _ KBFSOps = (*KBFSOpsStandard)(nil) +var ( + _ KBFSOps = (*KBFSOpsStandard)(nil) + _ kbfsInitWaiter = (*KBFSOpsStandard)(nil) +) const longOperationDebugDumpDuration = time.Minute @@ -82,7 +91,8 @@ const ctxKBFSOpsSkipEditHistoryBlock ctxKBFSOpsSkipEditHistoryBlockType = 1 // NewKBFSOpsStandard constructs a new KBFSOpsStandard object. // `initDone` should be closed when the rest of initialization (such -// as journal initialization) has completed. +// as journal initialization) has completed. If it fails instead, call +// initFailed. Call initReady as soon as config has what requests need. func NewKBFSOpsStandard( appStateUpdater env.AppStateUpdater, config Config, initDoneCh <-chan struct{}, @@ -97,6 +107,8 @@ func NewKBFSOpsStandard( opsByFav: make(map[favorites.Folder]*folderBranchOps), reIdentifyControlChan: make(chan chan<- struct{}), initDoneCh: initDoneCh, + initReadyCh: make(chan struct{}), + initFailedCh: make(chan struct{}), favs: NewFavorites(config), syncedTlfObservers: newSyncedTlfObserverList(), longOperationDebugDumper: NewImpatientDebugDumper( @@ -108,6 +120,62 @@ func NewKBFSOpsStandard( return kops } +func (fs *KBFSOpsStandard) signalInit(ready bool) { + fs.initMu.Lock() + defer fs.initMu.Unlock() + if fs.initSignaled { + return + } + fs.initSignaled = true + if ready { + close(fs.initReadyCh) + } else { + close(fs.initFailedCh) + } +} + +// initReady tells anything in waitForReady that config now has what requests +// need, ahead of the rest of init. +func (fs *KBFSOpsStandard) initReady() { + fs.signalInit(true) +} + +// initFailed tells anything waiting on init that it won't finish. No-op if +// initReady already ran: in-flight requests cannot be recalled. +func (fs *KBFSOpsStandard) initFailed() { + fs.signalInit(false) +} + +// ready reports, without blocking, whether init has set up what requests need. +func (fs *KBFSOpsStandard) ready() bool { + if fs.initDoneCh == nil { + return true + } + select { + case <-fs.initReadyCh: + return true + default: + return false + } +} + +// waitForReady blocks until init has set up what requests need, and returns +// errKBFSNotInitialized if init failed. A nil initDoneCh (KBFSOps built +// outside init, as in tests) counts as ready. +func (fs *KBFSOpsStandard) waitForReady(ctx context.Context) error { + if fs.initDoneCh == nil { + return nil + } + select { + case <-fs.initReadyCh: + return nil + case <-fs.initFailedCh: + return errKBFSNotInitialized{} + case <-ctx.Done(): + return ctx.Err() + } +} + func (fs *KBFSOpsStandard) markForReIdentifyIfNeededLoop() { maxValid := fs.config.TLFValidDuration() // Tests and some users fail to set this properly. @@ -2180,6 +2248,8 @@ func (fs *KBFSOpsStandard) initTlfsForEditHistories() { select { case <-fs.initDoneCh: + case <-fs.initFailedCh: + return case <-ctx.Done(): return } @@ -2267,6 +2337,8 @@ func (fs *KBFSOpsStandard) initSyncedTlfs() { select { case <-fs.initDoneCh: + case <-fs.initFailedCh: + return case <-ctx.Done(): return } diff --git a/go/kbfs/libkbfs/kbpki_client.go b/go/kbfs/libkbfs/kbpki_client.go index 80ea4d41c5f5..6cc40f823d55 100644 --- a/go/kbfs/libkbfs/kbpki_client.go +++ b/go/kbfs/libkbfs/kbpki_client.go @@ -53,12 +53,24 @@ func NewKBPKIClient( return &KBPKIClient{serviceOwner, log, cache} } +func (k *KBPKIClient) service() (KeybaseService, error) { + s := k.serviceOwner.KeybaseService() + if s == nil { + return nil, errKBFSNotInitialized{} + } + return s, nil +} + // GetCurrentSession implements the KBPKI interface for KBPKIClient. func (k *KBPKIClient) GetCurrentSession(ctx context.Context) ( idutil.SessionInfo, error, ) { + s, err := k.service() + if err != nil { + return idutil.SessionInfo{}, err + } const sessionID = 0 - return k.serviceOwner.KeybaseService().CurrentSession(ctx, sessionID) + return s.CurrentSession(ctx, sessionID) } // Resolve implements the KBPKI interface for KBPKIClient. @@ -67,7 +79,11 @@ func (k *KBPKIClient) Resolve( offline keybase1.OfflineAvailability) ( kbname.NormalizedUsername, keybase1.UserOrTeamID, error, ) { - return k.serviceOwner.KeybaseService().Resolve(ctx, assertion, offline) + s, err := k.service() + if err != nil { + return kbname.NormalizedUsername(""), keybase1.UserOrTeamID(""), err + } + return s.Resolve(ctx, assertion, offline) } // Identify implements the KBPKI interface for KBPKIClient. @@ -76,7 +92,11 @@ func (k *KBPKIClient) Identify( offline keybase1.OfflineAvailability) ( kbname.NormalizedUsername, keybase1.UserOrTeamID, error, ) { - return k.serviceOwner.KeybaseService().Identify( + s, err := k.service() + if err != nil { + return kbname.NormalizedUsername(""), keybase1.UserOrTeamID(""), err + } + return s.Identify( ctx, assertion, reason, offline) } @@ -432,20 +452,32 @@ func (k *KBPKIClient) CreateTeamTLF( // FavoriteAdd implements the KBPKI interface for KBPKIClient. func (k *KBPKIClient) FavoriteAdd(ctx context.Context, folder keybase1.FolderHandle) error { - return k.serviceOwner.KeybaseService().FavoriteAdd(ctx, folder) + s, err := k.service() + if err != nil { + return err + } + return s.FavoriteAdd(ctx, folder) } // FavoriteDelete implements the KBPKI interface for KBPKIClient. func (k *KBPKIClient) FavoriteDelete(ctx context.Context, folder keybase1.FolderHandle) error { - return k.serviceOwner.KeybaseService().FavoriteDelete(ctx, folder) + s, err := k.service() + if err != nil { + return err + } + return s.FavoriteDelete(ctx, folder) } // FavoriteList implements the KBPKI interface for KBPKIClient. func (k *KBPKIClient) FavoriteList(ctx context.Context) ( keybase1.FavoritesResult, error, ) { + s, err := k.service() + if err != nil { + return keybase1.FavoritesResult{}, err + } const sessionID = 0 - return k.serviceOwner.KeybaseService().FavoriteList(ctx, sessionID) + return s.FavoriteList(ctx, sessionID) } // Notify implements the KBPKI interface for KBPKIClient. diff --git a/go/kbfs/libkbfs/keybase_daemon_rpc.go b/go/kbfs/libkbfs/keybase_daemon_rpc.go index 870765f11274..f03592688725 100644 --- a/go/kbfs/libkbfs/keybase_daemon_rpc.go +++ b/go/kbfs/libkbfs/keybase_daemon_rpc.go @@ -81,6 +81,34 @@ func (k *KeybaseDaemonRPC) addKBFSProtocols() { k.AddProtocols(protocols) } +// gateOnKBFSReady wraps every method of the given protocols (SimpleFS, git, +// fs) so each request waits until init has set up what requests need, which +// is after the service connection is live. Each request is served on its own +// goroutine, so waiting doesn't block the connection. A request fails with +// errKBFSNotInitialized if init failed, or ends with the caller's context. +func gateOnKBFSReady(config Config, protocols []rpc.Protocol) []rpc.Protocol { + if len(protocols) == 0 { + return protocols + } + wrapped := make([]rpc.Protocol, 0, len(protocols)) + for _, p := range protocols { + methods := make(map[string]rpc.ServeHandlerDescription, len(p.Methods)) + for name, m := range p.Methods { + handler := m.Handler + m.Handler = func(ctx context.Context, arg any) (any, error) { + if err := waitForKBFSReady(ctx, config); err != nil { + return nil, err + } + return handler(ctx, arg) + } + methods[name] = m + } + p.Methods = methods + wrapped = append(wrapped, p) + } + return wrapped +} + // NewKeybaseDaemonRPC makes a new KeybaseDaemonRPC that makes RPC // calls using the socket of the given Keybase context. func NewKeybaseDaemonRPC(config Config, kbCtx Context, log logger.Logger, @@ -92,6 +120,9 @@ func NewKeybaseDaemonRPC(config Config, kbCtx Context, log logger.Logger, if debug { k.daemonLog.Configure("", true, "") } + // Handlers in OnConnect can run before this constructor returns, and + // KBPKI reaches the daemon through config. + config.SetKeybaseService(k) conn := NewSharedKeybaseConnection(kbCtx, config, k) k.fillClients(conn.GetClient()) k.shutdownFn = conn.Shutdown @@ -104,7 +135,7 @@ func NewKeybaseDaemonRPC(config Config, kbCtx Context, log logger.Logger, k.notifyService = newNotifyServiceHandler(config, log) k.addKBFSProtocols() - k.AddProtocols(additionalProtocols) + k.AddProtocols(gateOnKBFSReady(config, additionalProtocols)) return k } diff --git a/go/kbfs/libkbfs/keybase_daemon_rpc_test.go b/go/kbfs/libkbfs/keybase_daemon_rpc_test.go index 552e69a49e80..0dc390e5e60b 100644 --- a/go/kbfs/libkbfs/keybase_daemon_rpc_test.go +++ b/go/kbfs/libkbfs/keybase_daemon_rpc_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/golang/mock/gomock" + "github.com/keybase/client/go/kbfs/env" "github.com/keybase/client/go/kbfs/idutil" "github.com/keybase/client/go/kbfs/kbfscrypto" "github.com/keybase/client/go/kbfs/test/clocktest" @@ -55,6 +56,56 @@ func TestKeybaseDaemonRPCGetCurrentSessionCanceled(t *testing.T) { testRPCWithCanceledContext(t, serverConn, f) } +// NewKeybaseDaemonRPC gates the additional protocols (SimpleFS/git/fs) on +// init, since the service can call them as soon as the connection is up. +func TestKeybaseDaemonRPCGatesAdditionalProtocolsOnInit(t *testing.T) { + config := MakeTestConfigOrBust(t, "testuser") + origKBFSOps := config.KBFSOps() + initDoneCh := make(chan struct{}) + kbfsOps := NewKBFSOpsStandard(env.EmptyAppStateUpdater{}, config, initDoneCh) + config.SetKBFSOps(kbfsOps) + defer func() { + config.SetKBFSOps(origKBFSOps) + require.NoError(t, kbfsOps.Shutdown(context.Background())) + CheckConfigAndShutdown(context.Background(), t, config) + }() + + var calls int + daemon := NewKeybaseDaemonRPC( + config, newInitTestContext(t), logger.NewTestLogger(t), false, + []rpc.Protocol{newGatedTestProtocol(&calls)}) + defer daemon.Shutdown() + var handler func(context.Context, any) (any, error) + daemon.lock.Lock() + for _, p := range daemon.protocols { + if p.Name == "gatedTest" { + handler = p.Methods["method"].Handler + } + } + daemon.lock.Unlock() + require.NotNil(t, handler) + + // Init still running: the request waits, and the handler doesn't run. + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + _, err := handler(ctx, nil) + require.ErrorIs(t, err, context.DeadlineExceeded) + require.Zero(t, calls) + + // Init becoming ready mid-wait lets the request through, without the + // rest of init having finished. + go func() { + time.Sleep(50 * time.Millisecond) + kbfsOps.initReady() + }() + readyCtx, readyCancel := context.WithTimeout( + context.Background(), 5*time.Second) + defer readyCancel() + _, err = handler(readyCtx, nil) + require.NoError(t, err) + require.Equal(t, 1, calls) +} + // TODO: Add tests for Favorite* methods, too. type fakeKeybaseClient struct { diff --git a/go/kbfs/libkbfs/keybase_service_base.go b/go/kbfs/libkbfs/keybase_service_base.go index bf7c71dee74e..b60a05f45104 100644 --- a/go/kbfs/libkbfs/keybase_service_base.go +++ b/go/kbfs/libkbfs/keybase_service_base.go @@ -430,6 +430,39 @@ func (k *KeybaseServiceBase) ReachabilityChanged(ctx context.Context, return nil } +// errKBFSNotInitialized is returned to the service for requests that arrive +// before init has set up what they need, or after init failed. +type errKBFSNotInitialized struct{} + +func (errKBFSNotInitialized) Error() string { return "KBFS is not initialized yet" } + +// kbfsInitWaiter is implemented by KBFSOpsStandard. Init sets up the service +// connection before it has set up everything requests need, so handlers on +// that connection use it. +type kbfsInitWaiter interface { + waitForReady(ctx context.Context) error + ready() bool +} + +// waitForKBFSReady blocks until init has set up what requests need, and +// returns errKBFSNotInitialized if init failed. +func waitForKBFSReady(ctx context.Context, config Config) error { + if w, ok := config.KBFSOps().(kbfsInitWaiter); ok { + return w.waitForReady(ctx) + } + return nil +} + +// kbfsReady reports, without blocking, whether init has set up what requests +// need. Service-initiated requests check this rather than waiting, so none of +// them can block on an init that is itself waiting on the service. +func kbfsReady(config Config) bool { + if w, ok := config.KBFSOps().(kbfsInitWaiter); ok { + return w.ready() + } + return true +} + // StartReachability implements keybase1.ReachabilityInterface. func (k *KeybaseServiceBase) StartReachability(ctx context.Context) (res keybase1.Reachability, err error) { return k.CheckReachability(ctx) @@ -1340,6 +1373,9 @@ func (k *KeybaseServiceBase) FSEditListRequest(ctx context.Context, k.log) k.log.CDebugf(ctx, "Edit list request for %s (public: %t)", req.Folder.Name, !req.Folder.Private) + if !kbfsReady(k.config) { + return errKBFSNotInitialized{} + } tlfHandle, err := getHandleFromFolderName( ctx, k.config.KBPKI(), k.config.MDOps(), k.config, req.Folder.Name, !req.Folder.Private) @@ -1484,6 +1520,10 @@ func (k *KeybaseDaemonRPC) TeamTreeMembershipsDone(context.Context, func (k *KeybaseServiceBase) StartMigration(ctx context.Context, folder keybase1.Folder, ) (err error) { + // Before init is ready, MDServer is nil too; report the transient error. + if !kbfsReady(k.config) { + return errKBFSNotInitialized{} + } mdServer := k.config.MDServer() if mdServer == nil { return errors.New("no mdserver") @@ -1512,6 +1552,9 @@ func (k *KeybaseServiceBase) StartMigration(ctx context.Context, func (k *KeybaseServiceBase) FinalizeMigration(ctx context.Context, folder keybase1.Folder, ) (err error) { + if !kbfsReady(k.config) { + return errKBFSNotInitialized{} + } fav := favorites.NewFolderFromProtocol(folder) handle, err := GetHandleFromFolderNameAndType( ctx, k.config.KBPKI(), k.config.MDOps(), k.config, fav.Name, fav.Type) @@ -1549,6 +1592,9 @@ func (k *KeybaseServiceBase) GetTLFCryptKeys(ctx context.Context, return keybase1.GetTLFCryptKeysRes{}, err } + if !kbfsReady(k.config) { + return res, errKBFSNotInitialized{} + } tlfHandle, err := getHandleFromFolderName( ctx, k.config.KBPKI(), k.config.MDOps(), k.config, query.TlfName, false) if err != nil { @@ -1593,6 +1639,9 @@ func (k *KeybaseServiceBase) GetPublicCanonicalTLFNameAndID( return keybase1.CanonicalTLFNameAndIDWithBreaks{}, err } + if !kbfsReady(k.config) { + return res, errKBFSNotInitialized{} + } tlfHandle, err := getHandleFromFolderName( ctx, k.config.KBPKI(), k.config.MDOps(), k.config, query.TlfName, true /* public */) diff --git a/go/kbfs/libkbfs/keybase_service_util.go b/go/kbfs/libkbfs/keybase_service_util.go index 818714f4a9ea..2a55088ae6fa 100644 --- a/go/kbfs/libkbfs/keybase_service_util.go +++ b/go/kbfs/libkbfs/keybase_service_util.go @@ -143,7 +143,11 @@ func serviceLoggedOut(ctx context.Context, config Config) { } config.ResetCaches() config.UserHistory().Clear() - config.Chat().ClearCache() + // Init sets Chat after the service connection is live, so a logout can + // arrive before it exists. + if chat := config.Chat(); chat != nil { + chat.ClearCache() + } mdServer := config.MDServer() if mdServer != nil { mdServer.RefreshAuthToken(ctx) diff --git a/shared/desktop/app/html-root.desktop.tsx b/shared/desktop/app/html-root.desktop.tsx index 1f27d541afea..11674798adae 100644 --- a/shared/desktop/app/html-root.desktop.tsx +++ b/shared/desktop/app/html-root.desktop.tsx @@ -11,7 +11,7 @@ const fileRoot = `${htmlPrefix}${(`${distRoot}/`).replaceAll(path.sep, '/')}` // be loaded from file://). Cold dev + prod load the built html from file://. // Vite keeps the shells at their source paths in both cases, so the same // relative path is used for the http origin and the dist file root. -const devServerOrigin = 'http://localhost:4000' +export const devServerOrigin = 'http://localhost:4000' const htmlRelPath: Record = { main: 'desktop/renderer/main.html', remote: 'desktop/remote/remote.html', diff --git a/shared/desktop/app/main-window.desktop.tsx b/shared/desktop/app/main-window.desktop.tsx index 379dcaa229d3..6e30dafa0c67 100644 --- a/shared/desktop/app/main-window.desktop.tsx +++ b/shared/desktop/app/main-window.desktop.tsx @@ -7,7 +7,7 @@ import {showDevTools} from '@/local-debug' import {guiConfigFilename, isDarwin, isWindows, defaultUseNativeFrame} from '@/constants/platform' import logger from '@/logger' import debounce from 'lodash/debounce' -import {htmlURL, preloadPath} from './html-root.desktop' +import {devServerOrigin, htmlURL, preloadPath} from './html-root.desktop' import KB2 from '@/util/electron' const {env} = KB2.constants @@ -47,6 +47,17 @@ const setupDefaultSession = () => { } return callback(false) }) + + // In hot dev the renderer lives on the Vite http origin, so its XHRs to the + // KBFS http server (text previews) are cross-origin, and that server sends no + // CORS headers. Packaged builds load from file:// and don't need this. + if (__HOT__) { + ds.webRequest.onHeadersReceived({urls: ['http://127.0.0.1:*/files/*']}, (details, callback) => { + callback({ + responseHeaders: {...details.responseHeaders, 'Access-Control-Allow-Origin': [devServerOrigin]}, + }) + }) + } } const defaultWindowState = {