diff --git a/frontend/src/lib/itemStore.svelte.ts b/frontend/src/lib/itemStore.svelte.ts index 2332c3af8f..b9a2f0980b 100644 --- a/frontend/src/lib/itemStore.svelte.ts +++ b/frontend/src/lib/itemStore.svelte.ts @@ -180,8 +180,11 @@ class Entry { /** Replaced by an entry that moved onto its key: it no longer owns the row there, and a write * reaching its turn does nothing, as its handles show the item that replaced it. */ retired = false - /** The key a save of this entry is waiting to move onto, while it waits its turn there. */ - awaiting: ItemKey | undefined + /** The command running now, past its turn: what a move onto this key waits for. */ + running: Promise | undefined + /** The entries a save of this entry waits for before moving: a move skips waiting for any + * that waits for it in turn, which would never end. */ + waitingOn: Entry[] = [] handles = new Set>() stopWatch: (() => void) | undefined /** Fields a `patch` has put on the deployed side ahead of the server; a save landing @@ -255,25 +258,27 @@ class Entry { private run(fn: () => Promise): Promise { this.commands++ this.refs++ - const result = this.queue.then(fn).finally(() => { - this.commands-- - this.store.release(this) - }) + const result = this.queue + .then(() => this.turn(fn)) + .finally(() => { + this.commands-- + this.store.release(this) + }) this.queue = result.catch(() => {}) return result } - /** Let a move onto this key go next: resolves once the commands queued so far have run, and - * holds any queued after until `until`. */ - hold(until: Promise): Promise { - const turn = this.queue - this.queue = turn.then(() => until) - return turn - } - - /** A new entry at a key a move is heading for: its commands, its first load included, wait. */ - startAfter(move: Promise): void { - this.queue = move + /** A command's turn: after any other entry's move onto this key, whichever entry was here + * first, arrived during it or was put here by an earlier move. */ + private async turn(fn: () => Promise): Promise { + await this.store.moveSettled(this) + const running = fn() + this.running = running + try { + return await running + } finally { + if (this.running === running) this.running = undefined + } } /** Count a command as started now, ahead of its turn in the queue; returns its release. */ @@ -362,6 +367,7 @@ class Entry { /** `started`: the release `begin` handed out, for a save already counted as started. */ save(adapter: ItemAdapter, started?: () => void, asked = this.ask()): Promise { const body = async (): Promise => { + if (this.retired) return superseded const sent = asked ?? this.ask() if (sent === undefined) return { ok: false, error: this.error ?? 'Nothing to save' } const from = this.key @@ -406,7 +412,7 @@ class Entry { } } if (!started) return this.run(body) - const result = this.queue.then(body).finally(started) + const result = this.queue.then(() => this.turn(body)).finally(started) this.queue = result.catch(() => {}) return result } @@ -588,10 +594,8 @@ class Entry { type StoreInternals = { release(entry: Entry): void rekey(entry: Entry, from: ItemKey): void - claim( - key: ItemKey, - claimant: Entry - ): { turn: Promise; release: () => void } | undefined + claim(key: ItemKey, claimant: Entry): { turn: Promise; release: () => void } + moveSettled(entry: Entry): Promise } export type ItemHandle = { @@ -731,8 +735,8 @@ export type ItemAcquisition = { handle: ItemHandle; release(): void } export function createItemStore(ports: ItemRowPort) { const entries = new Map>() - /** Saves moving onto a key, by that key: each settles once its move is done. */ - const moves = new Map>() + /** The latest save moving onto a key, by that key, and when its move is done. */ + const moves = new Map; done: Promise }>() const internals: StoreInternals = { release(entry) { @@ -753,40 +757,54 @@ export function createItemStore(ports: ItemRowPort) { entries.set(to, entry) }, /** - * A save about to write the item at `key` and move onto it. It goes after whatever is queued - * at that key (the entry holding it, or a move already heading there), and until it is done - * everything else at that key waits: that holder's later commands, an entry acquired there - * meanwhile, another move. So one write to the item lands at a time, and whatever the move - * retires is idle by then. Not when the holder is itself waiting to move onto the claimant: - * each would wait for the other. + * A save about to write the item at `key` and move onto it. It goes after the command + * running at that key and the move already heading there, and every other command at that + * key waits for it when its turn comes (`moveSettled`), whichever entry issues it. So one + * write to the item lands at a time, and whatever the move retires is idle by then. It does + * not wait for an entry that waits for it in turn: each would wait for the other. */ claim(key, claimant) { const k = keyString(key) const holder = entries.get(k) - if (holder === claimant || (holder && waitsFor(holder, claimant))) return undefined + const previous = moves.get(k) let release!: () => void - const released = new Promise((r) => (release = r)) - const turn = holder ? holder.hold(released) : (moves.get(k) ?? Promise.resolve()) - moves.set(k, released) - claimant.awaiting = key + const move = { owner: claimant, done: new Promise((r) => (release = r)) } + moves.set(k, move) + const waits: Promise[] = [] + claimant.waitingOn = [] + if (previous && !waitsFor(previous.owner, claimant)) { + waits.push(previous.done) + claimant.waitingOn.push(previous.owner) + } + if (holder?.running && !waitsFor(holder, claimant)) { + waits.push(holder.running.catch(() => {})) + claimant.waitingOn.push(holder) + } return { - turn: turn.then(() => (claimant.awaiting = undefined)), + turn: Promise.all(waits).then(() => (claimant.waitingOn = [])), release() { release() - if (moves.get(k) === released) moves.delete(k) + if (moves.get(k) === move) moves.delete(k) } } + }, + async moveSettled(entry) { + let move = moves.get(keyString(entry.key)) + while (move && move.owner !== entry) { + await move.done + move = moves.get(keyString(entry.key)) + } } } function waitsFor(from: Entry, target: Entry): boolean { const seen = new Set>() - for (let e = from; e.awaiting && !seen.has(e); ) { + const pending = [from] + for (let e = pending.pop(); e; e = pending.pop()) { + if (e === target) return true + if (seen.has(e)) continue seen.add(e) - const next = entries.get(keyString(e.awaiting)) - if (!next) return false - if (next === target) return true - e = next + pending.push(...e.waitingOn) } return false } @@ -835,8 +853,6 @@ export function createItemStore(ports: ItemRowPort) { entry = new Entry(key, ports, internals) entry.settles = adapter.settles ?? false entry.absorbs = adapter.absorbs - const move = moves.get(k) - if (move) entry.startAfter(move) entries.set(k, entry) if (isTemporaryPath(key.path) && spec.template !== undefined) entry.initNew(spec.template) else void entry.load(adapter) diff --git a/frontend/src/lib/itemStore.test.ts b/frontend/src/lib/itemStore.test.ts index eb04b35eea..cbf24eb7d4 100644 --- a/frontend/src/lib/itemStore.test.ts +++ b/frontend/src/lib/itemStore.test.ts @@ -581,6 +581,44 @@ describe('item store: one entry per key', () => { expect(order).toEqual(['moved: moving']) expect(opened.value?.description).toBe('moving') }) + + it('holds an entry an earlier move put at a key while a later move is heading there', async () => { + const rows = fakeRows() + const store = createItemStore(rows.port) + const order: string[] = [] + const gate = deferred() + const b = { ...deployedRes, path: 'u/me/b' } + const creating = (write: (ctx: ItemWriteContext) => Promise) => { + const path = newItemPath() + return store.acquire( + { workspace: 'w', kind: 'resource', path }, + { workspace: 'w', path, template: b }, + adapter({}, write) + ).handle + } + const first = creating(async (ctx) => void order.push(ctx.value.description)) + const second = creating(async (ctx) => { + await gate.promise + order.push(ctx.value.description) + }) + await settle() + first.value = { ...b, description: 'first' } + second.value = { ...b, description: 'second' } + const firstMove = first.save() + const secondMove = second.save() + expect(await firstMove).toMatchObject({ ok: true, moved: true }) + + first.value = { ...b, description: 'saved again through the first' } + const later = first.save() + await settle() + expect(order).toEqual(['first']) + + gate.resolve() + expect(await secondMove).toMatchObject({ ok: true, moved: true }) + expect(await later).toMatchObject({ ok: false }) + expect(order).toEqual(['first', 'second']) + expect(first.value?.description).toBe('second') + }) }) describe('item store: conflicts', () => {