mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-14 21:26:15 -08:00
fix(services): Delete All drains queued writes before its reset; reserved writes skip its barrier
The previous commit raised Delete All's barrier inside the exclusive authority reset and drained queued writes there, but a queued deletion or import takes the shared side of that lock: the reset waited for it while it waited for the reset. Delete All drains the writes queued before it outside the lock again, then takes the reset, with its barrier up from the start. The deadlock with a Stalker repair (or Edit) that holds a row reservation and writes under it is broken on the other side: such a write does not wait for the barrier (holdsPlaylistAuthority, counted per reservation in this context), and the reset waits for the reservation as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1059224d79
commit
c4425c4c8d
4 files changed
+173
-76
No files matched your search
@@ -242,7 +242,10 @@ barrier and row reservation before its persisted-source preflight; contention
|
||||
or unavailable PWA locking declines repair without discovery, while an acquired
|
||||
reservation stays held through its conditional row transform. The reservation
|
||||
blocks every new authentication (including a URL edit with the same normalized
|
||||
fingerprint) and repair, and drains any work already in flight.
|
||||
fingerprint) and repair, and drains any work already in flight. Delete All
|
||||
drains the playlist writes queued before it, then takes the exclusive reset;
|
||||
later writes wait for the delete, except a write under a reservation this tab
|
||||
holds (`holdsPlaylistAuthority`), which the reset waits for in turn.
|
||||
When Save follows a lazy repair in the same tab, Edit publishes its local
|
||||
authentication owner first, drains that repair through the actual Web Lock
|
||||
request completion, and only then requests the row reservation itself. Repair
|
||||
|
||||
@@ -69,6 +69,17 @@ export async function runWithPlaylistAuthorityReset<T>(
|
||||
);
|
||||
}
|
||||
|
||||
/** Row reservations this browsing context holds now, per playlist. */
|
||||
const heldHere = new Map<string, number>();
|
||||
|
||||
/**
|
||||
* Whether this context holds a reservation on the playlist: a store-wide
|
||||
* reset waits for it, so its own writes must not wait for the reset.
|
||||
*/
|
||||
export function holdsPlaylistAuthority(playlistId: string): boolean {
|
||||
return (heldHere.get(playlistId) ?? 0) > 0;
|
||||
}
|
||||
|
||||
/** Tries to hold one origin-wide row reservation until its owner releases it. */
|
||||
export async function reservePlaylistAuthority(
|
||||
playlistId: string
|
||||
@@ -120,11 +131,18 @@ export async function reservePlaylistAuthority(
|
||||
}
|
||||
|
||||
let released = false;
|
||||
heldHere.set(playlistId, (heldHere.get(playlistId) ?? 0) + 1);
|
||||
return {
|
||||
status: 'acquired',
|
||||
release: () => {
|
||||
if (!released) {
|
||||
released = true;
|
||||
const count = (heldHere.get(playlistId) ?? 1) - 1;
|
||||
if (count > 0) {
|
||||
heldHere.set(playlistId, count);
|
||||
} else {
|
||||
heldHere.delete(playlistId);
|
||||
}
|
||||
releaseHeldLock();
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { EMPTY, firstValueFrom, from, of } from 'rxjs';
|
||||
import { DbStores, Playlist, PlaylistMeta } from '@iptvnator/shared/interfaces';
|
||||
import { reservePlaylistAuthority } from './playlist-cross-context-lock';
|
||||
import { PlaylistWriteConflict } from './playlist-storage-backend';
|
||||
import { PlaylistsService, resolvePlaylistParser } from './playlists.service';
|
||||
|
||||
@@ -1808,68 +1809,143 @@ describe('PlaylistsService', () => {
|
||||
expect(order).toEqual(['read', 'write', 'delete all', 'read', 'write']);
|
||||
});
|
||||
|
||||
it('lets a repair holding its reservation write before Delete All', async () => {
|
||||
let releaseRepair: () => void = () => undefined;
|
||||
const repairHeld = new Promise<void>(
|
||||
(resolve) => (releaseRepair = resolve)
|
||||
);
|
||||
const request = jest.fn(
|
||||
async <T>(
|
||||
name: string,
|
||||
options: LockOptions,
|
||||
callback: (lock: Lock | null) => Promise<T>
|
||||
) => {
|
||||
// The reset waits for the reservation a repair holds.
|
||||
if (options.mode === 'exclusive') {
|
||||
await repairHeld;
|
||||
describe('Delete All with cross-context locks', () => {
|
||||
/** Web Locks as far as these tests need: the barrier and free rows. */
|
||||
function fakeLocks() {
|
||||
let shared = 0;
|
||||
let exclusive = false;
|
||||
const waiting: (() => void)[] = [];
|
||||
const changed = () => waiting.splice(0).forEach((wake) => wake());
|
||||
const until = async (free: () => boolean) => {
|
||||
while (!free()) {
|
||||
await new Promise<void>((wake) => waiting.push(wake));
|
||||
}
|
||||
return callback({
|
||||
name,
|
||||
mode: options.mode ?? 'exclusive',
|
||||
} as Lock);
|
||||
}
|
||||
);
|
||||
Object.defineProperty(globalThis.navigator, 'locks', {
|
||||
configurable: true,
|
||||
value: { request },
|
||||
});
|
||||
const electron = {
|
||||
dbGetAppPlaylist: jest.fn(async () => ({
|
||||
_id: 'p9',
|
||||
title: 'P',
|
||||
favorites: [],
|
||||
})),
|
||||
dbGetAppPlaylists: jest.fn(async () => []),
|
||||
dbGetAppState: jest.fn(async (key: string) =>
|
||||
key === SQLITE_PLAYLIST_MIGRATION_FLAG ? '1' : null
|
||||
),
|
||||
dbSetAppState: jest.fn(),
|
||||
dbUpsertAppPlaylist: jest.fn(async () => undefined),
|
||||
dbUpsertAppPlaylists: jest.fn(),
|
||||
dbDeleteAllPlaylists: jest.fn(async () => undefined),
|
||||
};
|
||||
testWindow.electron = electron;
|
||||
const service = createService();
|
||||
};
|
||||
const request = jest.fn(
|
||||
async <T>(
|
||||
name: string,
|
||||
options: LockOptions,
|
||||
callback: (lock: Lock | null) => Promise<T>
|
||||
) => {
|
||||
const lock = {
|
||||
name,
|
||||
mode: options.mode ?? 'exclusive',
|
||||
} as Lock;
|
||||
if (name !== 'iptvnator:playlist-authority') {
|
||||
return callback(lock);
|
||||
}
|
||||
if (options.mode === 'shared') {
|
||||
await until(() => !exclusive);
|
||||
shared += 1;
|
||||
try {
|
||||
return await callback(lock);
|
||||
} finally {
|
||||
shared -= 1;
|
||||
changed();
|
||||
}
|
||||
}
|
||||
await until(() => !exclusive && shared === 0);
|
||||
exclusive = true;
|
||||
try {
|
||||
return await callback(lock);
|
||||
} finally {
|
||||
exclusive = false;
|
||||
changed();
|
||||
}
|
||||
}
|
||||
);
|
||||
Object.defineProperty(globalThis.navigator, 'locks', {
|
||||
configurable: true,
|
||||
value: { request },
|
||||
});
|
||||
}
|
||||
|
||||
let finishWrite: () => void = () => undefined;
|
||||
function store(slowWrites = false) {
|
||||
const electron = {
|
||||
dbGetAppPlaylist: jest.fn(async () => ({
|
||||
_id: 'p9',
|
||||
title: 'P',
|
||||
favorites: [],
|
||||
})),
|
||||
dbGetAppPlaylists: jest.fn(async () => []),
|
||||
dbGetAppState: jest.fn(async (key: string) =>
|
||||
key === SQLITE_PLAYLIST_MIGRATION_FLAG ? '1' : null
|
||||
),
|
||||
dbSetAppState: jest.fn(),
|
||||
dbUpsertAppPlaylist: jest.fn(() =>
|
||||
slowWrites
|
||||
? new Promise<void>(
|
||||
(resolve) => (finishWrite = resolve)
|
||||
)
|
||||
: Promise.resolve()
|
||||
),
|
||||
dbUpsertAppPlaylists: jest.fn(),
|
||||
dbDeletePlaylist: jest.fn(async () => undefined),
|
||||
dbDeleteAllPlaylists: jest.fn(async () => undefined),
|
||||
};
|
||||
testWindow.electron = electron;
|
||||
return electron;
|
||||
}
|
||||
|
||||
const settle = async () => {
|
||||
for (let i = 0; i < 50; i += 1) {
|
||||
for (let i = 0; i < 100; i += 1) {
|
||||
await Promise.resolve();
|
||||
}
|
||||
};
|
||||
|
||||
const removed = firstValueFrom(service.removeAll());
|
||||
// The repair's conditional row write, under its reservation.
|
||||
let written = false;
|
||||
const repair = firstValueFrom(
|
||||
service.transformPlaylistFavorites('p9', (f) => [...f, 'a'])
|
||||
).then(() => (written = true));
|
||||
await settle();
|
||||
it('lets a repair holding its reservation write first', async () => {
|
||||
fakeLocks();
|
||||
const electron = store();
|
||||
const service = createService();
|
||||
const reservation = await reservePlaylistAuthority('p9');
|
||||
if (reservation.status !== 'acquired') {
|
||||
throw new Error('expected a reservation');
|
||||
}
|
||||
|
||||
expect(written).toBe(true);
|
||||
expect(electron.dbDeleteAllPlaylists).not.toHaveBeenCalled();
|
||||
releaseRepair();
|
||||
await repair;
|
||||
await removed;
|
||||
expect(electron.dbDeleteAllPlaylists).toHaveBeenCalled();
|
||||
const removed = firstValueFrom(service.removeAll());
|
||||
// The repair's conditional row write, under its reservation.
|
||||
let written = false;
|
||||
const repair = firstValueFrom(
|
||||
service.transformPlaylistFavorites('p9', (f) => [...f, 'a'])
|
||||
).then(() => (written = true));
|
||||
await settle();
|
||||
|
||||
expect(written).toBe(true);
|
||||
expect(electron.dbDeleteAllPlaylists).not.toHaveBeenCalled();
|
||||
reservation.release();
|
||||
await repair;
|
||||
await removed;
|
||||
expect(electron.dbDeleteAllPlaylists).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('lets a deletion queued behind a slow write finish first', async () => {
|
||||
fakeLocks();
|
||||
const electron = store(true);
|
||||
const service = createService();
|
||||
|
||||
const slow = firstValueFrom(
|
||||
service.transformPlaylistFavorites('p9', (f) => [...f, 'a'])
|
||||
);
|
||||
await settle();
|
||||
const deleted = firstValueFrom(service.deletePlaylist('p9'));
|
||||
let done = false;
|
||||
const removed = firstValueFrom(service.removeAll()).then(
|
||||
() => (done = true)
|
||||
);
|
||||
await settle();
|
||||
finishWrite();
|
||||
await settle();
|
||||
|
||||
// The deletion needs the shared lock: it cannot wait for a
|
||||
// reset that waits for it.
|
||||
expect(done).toBe(true);
|
||||
await slow;
|
||||
await deleted;
|
||||
await removed;
|
||||
expect(electron.dbDeletePlaylist).toHaveBeenCalledWith('p9');
|
||||
expect(electron.dbDeleteAllPlaylists).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it('runs a mutation again on a fresh read when the server refused it', async () => {
|
||||
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
import { PLAYLIST_DELETE_CLEANUP } from './playlist-delete-cleanup.token';
|
||||
import {
|
||||
runWithPlaylistAuthorityMutation,
|
||||
holdsPlaylistAuthority,
|
||||
runWithPlaylistAuthorityReset,
|
||||
} from './playlist-cross-context-lock';
|
||||
import {
|
||||
@@ -431,7 +432,11 @@ export class PlaylistsService {
|
||||
return defer(() => {
|
||||
const previous = Promise.all([
|
||||
this.playlistWriteQueues.get(playlistId),
|
||||
this.deletingAll,
|
||||
// A write under a reservation held here (a Stalker repair or
|
||||
// Edit) cannot wait for Delete All: its reset waits for it.
|
||||
holdsPlaylistAuthority(playlistId)
|
||||
? undefined
|
||||
: this.deletingAll,
|
||||
]);
|
||||
const next = previous.then(() => retryOnConflict(operation));
|
||||
const tail = next.then(
|
||||
@@ -1342,25 +1347,20 @@ export class PlaylistsService {
|
||||
}
|
||||
|
||||
removeAll(): Observable<void> {
|
||||
// Inside the exclusive authority reset: a Stalker repair already
|
||||
// holding its reservation finishes its write first (blocking it here
|
||||
// would deadlock). Then writes queued so far land, and new ones wait
|
||||
// for the delete.
|
||||
return defer(() =>
|
||||
runWithPlaylistAuthorityReset(async () => {
|
||||
const pending = [...this.playlistWriteQueues.values()];
|
||||
let deleted: () => void = () => undefined;
|
||||
this.deletingAll = new Promise<void>(
|
||||
(resolve) => (deleted = resolve)
|
||||
);
|
||||
try {
|
||||
await Promise.all(pending);
|
||||
await this.deleteAllRows();
|
||||
} finally {
|
||||
deleted();
|
||||
}
|
||||
})
|
||||
);
|
||||
// Writes queued so far land first, outside the reset (some take the
|
||||
// shared side of its lock); new ones wait for the delete, except
|
||||
// those under a reservation held here (serializePlaylistWrite).
|
||||
return defer(() => {
|
||||
const pending = [...this.playlistWriteQueues.values()];
|
||||
const run = Promise.all(pending).then(() =>
|
||||
runWithPlaylistAuthorityReset(() => this.deleteAllRows())
|
||||
);
|
||||
this.deletingAll = run.then(
|
||||
() => undefined,
|
||||
() => undefined
|
||||
);
|
||||
return run;
|
||||
});
|
||||
}
|
||||
|
||||
private async deleteAllRows(): Promise<void> {
|
||||
|
||||
Reference in new issue
Block a user