lockfile.Modified() is designed incorrectly

[mtrmac]

The lock object (intentionally) is shared across the whole process, and apparently intended to allow multiple in-process callers to work correctly, similarly to how it work with multiple processes using the same lock.

But Modified() is an “edge-triggered” signal, which returns true once after observing the last-writer ID to be updated.

If there are multiple stores pointing at the same lockfile object, only one of them will see Modified to be set to true, and the others will miss the update.

AFAICS the lockfile.lw state should, instead, be maintained in callers of lockfile.Modified, not in the lock object itself.

---

Right now, this is probably OK enough; we can have multiple stores in a single process pointing at the same lock file simultaneously (via something like

storage/store.go

Line 867 in 74e3743

s.layerStore = nil

, and there’s a recent-ish panic report suggesting this code path), but we only call Modified at the start of the operation, and when creating a store object. So if the linked code path triggers abandoning an existing store object, that object has probably (not certainly) already called Modified, and doesn’t care any more and won’t trigger the lockfile update any more; and the new replacement store object is loading data anyway.