From d5619d5a921136645ecad43647214cfa0757e6b7 Mon Sep 17 00:00:00 2001 From: yetone Date: Mon, 5 Oct 2026 10:55:12 +0800 Subject: [PATCH] =?UTF-8?q?windows:=20an=20account=20list=20read=20while?= =?UTF-8?q?=20another=20magpie=20writes=20it=20is=20waited=20for,=20not=20?= =?UTF-8?q?taken=20for=20no=20accounts=20(Packing1=20on=20Discord,=20Windo?= =?UTF-8?q?ws=2011:=20=E6=97=A9=E4=B8=8A=E6=9B=B4=E6=96=B0=E9=87=8D?= =?UTF-8?q?=E5=90=AF=E5=90=8E=E4=B9=8B=E5=89=8D=E7=99=BB=E5=BD=95=E7=9A=84?= =?UTF-8?q?Antigravity=E8=AE=A2=E9=98=85=E4=BE=9B=E5=BA=94=E5=95=86?= =?UTF-8?q?=E5=92=8C=E8=B4=A6=E5=8F=B7=E9=83=BD=E6=B2=A1=E4=BA=86=20?= =?UTF-8?q?=E2=80=A6=20=E6=95=B4=E8=A1=8C=E9=83=BD=E6=B2=A1=E4=BA=86?= =?UTF-8?q?=EF=BC=8C=E4=BE=9B=E5=BA=94=E5=95=86=E9=87=8C=E9=9D=A2antigravi?= =?UTF-8?q?ty=E7=9B=B4=E6=8E=A5=E6=B6=88=E5=A4=B1=E4=BA=86)=20=E2=80=94=20?= =?UTF-8?q?on=20Windows=20a=20file=20being=20renamed=20over=20(every=20wri?= =?UTF-8?q?te=20magpie=20makes:=20a=20temp=20file=20renamed=20onto=20it)?= =?UTF-8?q?=20is,=20for=20a=20moment=20of=20up=20to=20~2ms,=20"The=20syste?= =?UTF-8?q?m=20cannot=20find=20the=20file=20specified"=20to=20anyone=20els?= =?UTF-8?q?e=20looking=20at=20it,=20with=20MoveFileEx=20and=20with=20POSIX?= =?UTF-8?q?-semantics=20renames=20alike.=20filememo.Read=20gave=20that=20b?= =?UTF-8?q?ack=20as=20an=20error,=20readLogins=20as=20no=20accounts,=20and?= =?UTF-8?q?=20whatever=20change=20came=20next=20(any=20account's=20read-mo?= =?UTF-8?q?dify-write)=20wrote=20the=20empty=20list=20back:=20every=20buil?= =?UTF-8?q?t-in=20account=20gone,=20its=20subscription=20with=20it.=20Meas?= =?UTF-8?q?ured=20on=20Windows=2011=20(10.0.0.132):=20one=20process=20repl?= =?UTF-8?q?acing=20a=20200=20KB=20file,=20another=20stat-ing=20and=20readi?= =?UTF-8?q?ng=20it,=20saw=20the=20file=20"not=20there"=20in=20a=20few=20wi?= =?UTF-8?q?ndows=20each=2020=20s.=20Now=20steady.Stat=20and=20steady.ReadT?= =?UTF-8?q?here=20wait=20out=20a=20moment=20of=20a=20file=20not=20there=20?= =?UTF-8?q?(15=20tries=20a=20millisecond=20apart,=20Windows=20only),=20and?= =?UTF-8?q?=20filememo=20looks=20through=20them,=20remembering=20a=20file?= =?UTF-8?q?=20found=20not=20there=20so=20one=20that=20really=20isn't=20cos?= =?UTF-8?q?ts=20the=20wait=20once.=20A=20magpie=20started=20by=20an=20upda?= =?UTF-8?q?te=20now=20waits=20for=20the=20one=20it=20replaces=20before=20i?= =?UTF-8?q?ts=20start-up=20moves=20write=20any=20file=20(AwaitPredecessor?= =?UTF-8?q?=20at=20the=20top=20of=20run,=20not=20only=20in=20the=20GUI).?= =?UTF-8?q?=20TestLoginsKeptWhileAnotherMagpieWrites=20(Windows:=20two=20w?= =?UTF-8?q?riters=20replacing=20logins.json,=20readLogins=20never=20withou?= =?UTF-8?q?t=20the=20Antigravity=20account)=20failed=201=20in=203=20and=20?= =?UTF-8?q?2=20in=208=20runs=20on=20the=20old=20code=20("read=20as=20no=20?= =?UTF-8?q?accounts=208=20times"),=20passed=2011=20of=2011=20on=20this;=20?= =?UTF-8?q?filememo's=20TestReadWhileReplaced=20the=20same=20(1180=20misse?= =?UTF-8?q?s=20on=20the=20old=20code);=20TestStatWaitsOutAReplace=20everyw?= =?UTF-8?q?here.=20A=20real=20update=20in=20a=20sandbox=20home=20on=20the?= =?UTF-8?q?=20Windows=20box,=20magpie-cli=200.1.880=20=E2=86=92=200.1.942?= =?UTF-8?q?=20from=20the=20release,=20kept=20the=20account=20and=20logins.?= =?UTF-8?q?json=20byte=20for=20byte.=20go=20vet=20(darwin,=20and=20windows?= =?UTF-8?q?=20nogui),=20builds=20(darwin,=20GOOS=3Dlinux=20nogui,=20GOOS?= =?UTF-8?q?=3Dwindows)=20and=20go=20test=20-tags=20nogui=20-count=3D1=20./?= =?UTF-8?q?...=20under=20a=20temp=20HOME=20pass.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/filememo/filememo.go | 36 ++++++++++- internal/filememo/filememo_windows_test.go | 60 +++++++++++++++++++ internal/provider/logins_windows_test.go | 70 ++++++++++++++++++++++ internal/steady/steady.go | 61 +++++++++++++++++-- internal/steady/steady_test.go | 43 +++++++++++++ main.go | 3 + 6 files changed, 267 insertions(+), 6 deletions(-) create mode 100644 internal/filememo/filememo_windows_test.go create mode 100644 internal/provider/logins_windows_test.go create mode 100644 internal/steady/steady_test.go 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