diff --git a/internal/filememo/filememo.go b/internal/filememo/filememo.go index fe4c4551..c4a22817 100644 --- a/internal/filememo/filememo.go +++ b/internal/filememo/filememo.go @@ -4,6 +4,8 @@ package filememo import ( "bytes" + "errors" + "io/fs" "os" "sync" "time" @@ -37,6 +39,10 @@ var ( // on by every write magpie makes (Forget) holds int gen uint64 + // gone is the files found not there last time looked at, not waited + // on again (steady.Stat): one there before and not now may be being + // replaced (Windows), and is + gone = map[string]bool{} ) // heldFor is how long a file found unchanged is taken as unchanged, while @@ -82,7 +88,7 @@ func Read[T any](kind, path string, parse func([]byte) (T, error)) (T, error) { } g := gen mu.Unlock() - fi, err := os.Stat(path) + fi, err := statFile(path) if err != nil { return zero, err } @@ -95,7 +101,7 @@ func Read[T any](kind, path string, parse func([]byte) (T, error)) (T, error) { mu.Unlock() return e.v.(T), nil } - b, err := steady.ReadFile(path) + b, err := steady.ReadThere(path) if err != nil { return zero, err } @@ -123,3 +129,29 @@ func Read[T any](kind, path string, parse func([]byte) (T, error)) (T, error) { mu.Unlock() return v, nil } + +// statFile is os.Stat of path, waiting out a moment of it not there +// (another process replacing it, on Windows) unless it wasn't there last +// time either: taken for gone, a file of accounts read as no accounts, and +// the next write left them all out (Packing1 on Discord, Windows 11: +// Antigravity's account gone after an update). +func statFile(path string) (os.FileInfo, error) { + mu.Lock() + wasGone := gone[path] + mu.Unlock() + var fi os.FileInfo + var err error + if wasGone { + fi, err = os.Stat(path) + } else { + fi, err = steady.Stat(path) + } + mu.Lock() + if errors.Is(err, fs.ErrNotExist) { + gone[path] = true + } else { + delete(gone, path) + } + mu.Unlock() + return fi, err +} diff --git a/internal/filememo/filememo_windows_test.go b/internal/filememo/filememo_windows_test.go new file mode 100644 index 00000000..f1f539f3 --- /dev/null +++ b/internal/filememo/filememo_windows_test.go @@ -0,0 +1,60 @@ +package filememo + +import ( + "bytes" + "os" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/yetone/magpie/internal/steady" +) + +// Another magpie replacing the file (a write is a temp file renamed over +// it) has Windows say, for a moment, that the file isn't there at all: read +// so, a file of accounts was no accounts, and the next write left them out +// (Packing1 on Discord, Windows 11: Antigravity's provider and its account +// gone after an update). Read waits that moment out. +func TestReadWhileReplaced(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "logins.json") + body := bytes.Repeat([]byte("x"), 100_000) + os.WriteFile(p, body, 0o600) + var stop atomic.Bool + done := make(chan struct{}) + for range 2 { + go func() { + defer func() { done <- struct{}{} }() + for !stop.Load() { + tmp, err := os.CreateTemp(dir, ".logins.json.*") + if err != nil { + continue + } + tmp.Write(body) + tmp.Close() + if steady.Rename(tmp.Name(), p) != nil { + os.Remove(tmp.Name()) + } + } + }() + } + defer func() { + stop.Store(true) + <-done + <-done + }() + parse := func(b []byte) (int, error) { return len(b), nil } + misses := 0 + for end := time.Now().Add(5 * time.Second); time.Now().Before(end); { + if n, err := Read("replaced", p, parse); err != nil || n != len(body) { + misses++ + if misses < 4 { + t.Logf("read %d bytes, %v", n, err) + } + } + } + if misses > 0 { + t.Fatalf("the file read as not there %d times while replaced", misses) + } +} diff --git a/internal/provider/logins_windows_test.go b/internal/provider/logins_windows_test.go new file mode 100644 index 00000000..d1ffac21 --- /dev/null +++ b/internal/provider/logins_windows_test.go @@ -0,0 +1,70 @@ +package provider + +import ( + "os" + "path/filepath" + "slices" + "sync/atomic" + "testing" + "time" + + "github.com/yetone/magpie/internal/steady" +) + +// Another magpie writing logins.json (a temp file renamed over it: the one +// an update is replacing, a second one started) has Windows say, for a +// moment, that it isn't there: read so, it was no accounts, and the write +// that followed left every account out (Packing1 on Discord, Windows 11: +// 「整行都没了,供应商里面antigravity直接消失了」 after an update). +func TestLoginsKeptWhileAnotherMagpieWrites(t *testing.T) { + googleSandbox(t, &fakeGoogle{}) + if err := addGoogleLogin("antigravity", "a@x.com", "", googleAuth{RefreshToken: "1//a", Project: "pa"}); err != nil { + t.Fatal(err) + } + p := loginsPath() + body, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + var stop atomic.Bool + done := make(chan struct{}) + for range 2 { + go func() { + defer func() { done <- struct{}{} }() + for !stop.Load() { + tmp, err := os.CreateTemp(filepath.Dir(p), ".logins.json.*") + if err != nil { + continue + } + tmp.Write(body) + tmp.Close() + if steady.Rename(tmp.Name(), p) != nil { + os.Remove(tmp.Name()) + } + } + }() + } + has := func(ls []savedLogin) bool { + return slices.ContainsFunc(ls, func(l savedLogin) bool { return l.Agent == "antigravity" && l.User == "a@x.com" }) + } + misses := 0 + for end := time.Now().Add(5 * time.Second); time.Now().Before(end); { + loginsMu.Lock() + if ls := readLogins(); !has(ls) { + misses++ + // what every change to an account does next: the list read, + // changed, written back + _ = writeLogins(ls) + } + loginsMu.Unlock() + } + stop.Store(true) + <-done + <-done + if !has(readLogins()) { + t.Fatalf("the Antigravity account is gone from logins.json (read as none %d times)", misses) + } + if misses > 0 { + t.Fatalf("logins.json read as no accounts %d times while another magpie wrote it", misses) + } +} diff --git a/internal/steady/steady.go b/internal/steady/steady.go index be9c1099..78d37f24 100644 --- a/internal/steady/steady.go +++ b/internal/steady/steady.go @@ -2,8 +2,11 @@ // replacing at that moment. On Windows a file being renamed over refuses // to open, and one held open refuses to be renamed over, both for a // moment ("Access is denied"): read as none, a file of accounts is no -// accounts, and the next write leaves them out. Elsewhere a rename is -// atomic and these are os.ReadFile and os.Rename. +// accounts, and the next write leaves them out. A file being renamed over +// is also, for a moment (two milliseconds seen), not there at all, to +// another process or thread: Stat and ReadThere wait that out for a file +// that was there. Elsewhere a rename is atomic and these are os.Stat, +// os.ReadFile and os.Rename. package steady import ( @@ -21,20 +24,70 @@ const ( pause = 10 * time.Millisecond ) +// goneTries and gonePause bound the wait on a file not there to 15ms or +// so: the moment a rename over it leaves is a couple of milliseconds, and +// a file that is really not there is waited on that long. +const ( + goneTries = 15 + gonePause = time.Millisecond +) + +// windows is whether files behave as Windows' do; a test sets it. +var windows = runtime.GOOS == "windows" + +// stat and readFile are os.Stat and os.ReadFile; a test stands in for them. +var ( + stat = os.Stat + readFile = os.ReadFile +) + func retry(f func() error) error { err := f() - for i := 0; err != nil && runtime.GOOS == "windows" && !errors.Is(err, fs.ErrNotExist) && i < tries; i++ { + for i := 0; err != nil && windows && !errors.Is(err, fs.ErrNotExist) && i < tries; i++ { time.Sleep(pause) err = f() } return err } +// retryGone is retry, waiting out a moment of the file not there too. +func retryGone(f func() error) error { + err := retry(f) + for i := 0; err != nil && windows && errors.Is(err, fs.ErrNotExist) && i < goneTries; i++ { + time.Sleep(gonePause) + err = retry(f) + } + return err +} + +// Stat is os.Stat of a file that was there: one being replaced is waited +// for, so it isn't taken for gone. A file not there costs the wait, so a +// caller asking again remembers it gone (filememo does). +func Stat(p string) (fs.FileInfo, error) { + var fi fs.FileInfo + err := retryGone(func() (err error) { + fi, err = stat(p) + return err + }) + return fi, err +} + +// ReadThere is ReadFile of a file just found there (Stat): not there now is +// it being replaced, waited out. +func ReadThere(p string) ([]byte, error) { + var b []byte + err := retryGone(func() (err error) { + b, err = readFile(p) + return err + }) + return b, err +} + // ReadFile is os.ReadFile, waiting out a file being replaced. func ReadFile(p string) ([]byte, error) { var b []byte err := retry(func() (err error) { - b, err = os.ReadFile(p) + b, err = readFile(p) return err }) return b, err diff --git a/internal/steady/steady_test.go b/internal/steady/steady_test.go new file mode 100644 index 00000000..63bd793d --- /dev/null +++ b/internal/steady/steady_test.go @@ -0,0 +1,43 @@ +package steady + +import ( + "errors" + "io/fs" + "os" + "runtime" + "testing" +) + +// as Windows has it: a file being renamed over is, for a moment, not there +func TestStatWaitsOutAReplace(t *testing.T) { + windows = true + defer func() { windows, stat, readFile = runtime.GOOS == "windows", os.Stat, os.ReadFile }() + missing := 3 + stat = func(p string) (fs.FileInfo, error) { + if missing > 0 { + missing-- + return nil, fs.ErrNotExist + } + return os.Stat(p) + } + p := t.TempDir() + if _, err := Stat(p); err != nil { + t.Fatalf("a file being replaced taken for gone: %v", err) + } + missing = 3 + readFile = func(p string) ([]byte, error) { + if missing > 0 { + missing-- + return nil, &fs.PathError{Op: "open", Path: p, Err: fs.ErrNotExist} + } + return []byte("accounts"), nil + } + if b, err := ReadThere(p); err != nil || string(b) != "accounts" { + t.Fatalf("read %q, %v", b, err) + } + // one really gone is said so, after the wait + stat = os.Stat + if _, err := Stat(p + "-none"); !errors.Is(err, fs.ErrNotExist) { + t.Fatalf("a file not there: %v", err) + } +} diff --git a/main.go b/main.go index a455264b..d8124da7 100644 --- a/main.go +++ b/main.go @@ -190,6 +190,9 @@ func run(args []string) error { // (agent.DisconnectPreview) return agent.DryRun(args[1]) } + // started by an update, the magpie it replaces goes first: the moves + // below write the files it may still be writing + update.AwaitPredecessor() makeDirs() settings.Migrate() // the providers and settings read once for every agent's fields, which