Skip to content

Commit 988ec9c

Browse files
authored
Merge pull request #2302 from dexie/liz/fix-blobsave-hooks-psd-context
fix(dexie-cloud): Optimize and harden runtime Blob Resolving
2 parents 59091b7 + b74758f commit 988ec9c

6 files changed

Lines changed: 335 additions & 210 deletions

File tree

addons/dexie-cloud/src/middlewares/blobResolveMiddleware.ts

Lines changed: 24 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@
88
* Uses Dexie.waitFor() only for explicit rw transactions to keep them alive.
99
* For readonly or implicit transactions, resolves directly (no waitFor needed).
1010
*
11-
* Resolved blobs are queued for saving via BlobSavingQueue, which uses
12-
* setTimeout(fn, 0) to completely isolate from Dexie's transaction context.
13-
* Each blob is saved atomically using Table.update() with its keyPath to
14-
* avoid race conditions with other property changes.
11+
* Resolved blobs are persisted via db.blobDownloadTracker.enqueueSave(),
12+
* which internally uses a queue that runs in a fresh JS task to completely
13+
* isolate from Dexie's transaction context. Each blob is saved atomically
14+
* using Table.update() with its keyPath to avoid race conditions with other
15+
* property changes.
1516
*
1617
* Blob downloads use Authorization header (same as sync) via the server
1718
* proxy endpoint: GET /blob/{ref}
@@ -34,7 +35,6 @@ import {
3435
resolveAllBlobRefs,
3536
ResolvedBlob,
3637
} from '../sync/blobResolve';
37-
import { BlobSavingQueue } from '../sync/BlobSavingQueue';
3838
import { TXExpandos } from '../types/TXExpandos';
3939
import { UserLogin } from '../dexie-cloud-client';
4040

@@ -46,9 +46,6 @@ export function createBlobResolveMiddleware(
4646
name: 'blobResolve',
4747
level: 2, // Run above cache (0) and other middlewares (1) to resolve BlobRefs from cached data
4848
create(downlevelDatabase: DBCore): DBCore {
49-
// Create a single queue instance for this database
50-
const blobSavingQueue = new BlobSavingQueue(db);
51-
5249
return {
5350
...downlevelDatabase,
5451
table(tableName: string): DBCoreTable {
@@ -82,7 +79,6 @@ export function createBlobResolveMiddleware(
8279
req.trans,
8380
req.key,
8481
result,
85-
blobSavingQueue,
8682
db
8783
);
8884
}
@@ -112,7 +108,6 @@ export function createBlobResolveMiddleware(
112108
req.trans,
113109
req.keys[index],
114110
result,
115-
blobSavingQueue,
116111
db
117112
);
118113
}
@@ -147,7 +142,6 @@ export function createBlobResolveMiddleware(
147142
req.trans,
148143
undefined,
149144
item,
150-
blobSavingQueue,
151145
db
152146
);
153147
}
@@ -168,12 +162,7 @@ export function createBlobResolveMiddleware(
168162
if (!cursor) return cursor; // No results, so no resolution needed
169163
if (!req.values) return cursor; // No values requested, so no resolution needed
170164
if (!dbUrl) return cursor; // No database URL configured, can't resolve blobs
171-
return createBlobResolvingCursor(
172-
cursor,
173-
downlevelTable,
174-
blobSavingQueue,
175-
db
176-
);
165+
return createBlobResolvingCursor(cursor, downlevelTable, db);
177166
});
178167
},
179168
};
@@ -196,7 +185,6 @@ export function createBlobResolveMiddleware(
196185
function createBlobResolvingCursor(
197186
cursor: DBCoreCursor,
198187
table: DBCoreTable,
199-
blobSavingQueue: BlobSavingQueue,
200188
db: DexieCloudDB
201189
): DBCoreCursor {
202190
// Create wrapped cursor using Object.create() - inherits everything.
@@ -206,11 +194,15 @@ function createBlobResolvingCursor(
206194
// throws "Illegal invocation" in Chrome 146+.
207195
const wrappedCursor = Object.create(cursor, {
208196
key: {
209-
get() { return cursor.key; },
197+
get() {
198+
return cursor.key;
199+
},
210200
configurable: true,
211201
},
212202
primaryKey: {
213-
get() { return cursor.primaryKey; },
203+
get() {
204+
return cursor.primaryKey;
205+
},
214206
configurable: true,
215207
},
216208
value: {
@@ -233,7 +225,6 @@ function createBlobResolvingCursor(
233225
cursor.trans,
234226
cursor.primaryKey,
235227
rawValue,
236-
blobSavingQueue,
237228
db,
238229
true
239230
).then(
@@ -276,7 +267,6 @@ function resolveAndSave(
276267
trans: DBCoreTransaction,
277268
pKey: any | undefined, // optional. If missing, tries to extract from object using primary key path
278269
obj: any,
279-
blobSavingQueue: BlobSavingQueue,
280270
db: DexieCloudDB,
281271
isCursorValue: boolean = false // Flag to indicate if we're resolving a cursor value (which may not have a primary key)
282272
): Promise<any> {
@@ -328,23 +318,18 @@ function resolveAndSave(
328318
: undefined;
329319

330320
if (key !== undefined) {
331-
// Queue each resolved blob individually for atomic update
332-
// This uses setTimeout(fn, 0) to completely isolate from
333-
// Dexie's transaction context (avoids inheriting PSD)
334-
if (isReadonly) {
335-
blobSavingQueue.saveBlobs(table.name, key, resolvedBlobs);
336-
} else {
337-
// For rw transactions, we can save directly without queueing
338-
// since we're still in the same transaction context
339-
table
340-
.mutate({ type: 'put', keys: [key], values: [resolved], trans })
341-
.catch((err) => {
342-
console.error(
343-
`Failed to save resolved blob on ${table.name}:${key}:`,
344-
err
345-
);
346-
});
347-
}
321+
// Hand off persistence to the tracker. The tracker owns an
322+
// internal save-queue that runs in a fresh JS task (setTimeout 0)
323+
// — completely outside any PSD context, so opening a Dexie rw
324+
// transaction there is always safe regardless of the calling
325+
// context. The tracker also keeps the in-flight download cache
326+
// alive until the save completes, so concurrent readers piggyback
327+
// on the already-downloaded data instead of refetching.
328+
db.blobDownloadTracker.enqueueSave(table.name, key, resolvedBlobs);
329+
} else if (resolvedBlobs.length > 0) {
330+
// No primary key — we can't persist. Release the in-flight cache
331+
// entries explicitly so they don't leak.
332+
db.blobDownloadTracker.releaseRefs(resolvedBlobs.map((b) => b.ref));
348333
}
349334

350335
return resolved;
Lines changed: 160 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,78 +1,193 @@
11
import type { DexieCloudDB } from '../db/DexieCloudDB';
2-
import { BlobRef } from './blobResolve';
2+
import { BlobRef, ResolvedBlob } from './blobResolve';
3+
import { BlobSavingQueue } from './BlobSavingQueue';
34
import { loadCachedAccessToken } from './loadCachedAccessToken';
45

56
/**
6-
* Deduplicates in-flight blob downloads.
7+
* Owns the full lifecycle of downloaded blobs:
8+
* 1. Deduplicates concurrent downloads for the same ref.
9+
* 2. Bounds the number of concurrent network fetches (MAX_CONCURRENT)
10+
* so that ad-hoc reads can't starve the HTTP connection pool. Calls
11+
* beyond the cap queue in FIFO order as slots free. The slot is held
12+
* only for the duration of the fetch — NOT until persistence — to
13+
* avoid deadlocks when a single object contains more blob refs than
14+
* MAX_CONCURRENT (a sequential resolver would otherwise hold every
15+
* slot itself while waiting for the next).
16+
* 3. Keeps the in-flight promise alive after the network fetch completes,
17+
* until the blob has been persisted back to IndexedDB. This way,
18+
* readers that ask for the same ref while it is queued for saving
19+
* can piggyback on the existing promise instead of refetching.
20+
* In-flight membership and slot ownership are independent: a piggyback
21+
* reader consumes neither a slot nor extra memory beyond the existing
22+
* cached Uint8Array.
23+
* 4. Persists resolved blobs via an internal BlobSavingQueue, and
24+
* releases the in-flight entry when persistence completes.
725
*
8-
* Both the blob-resolve middleware and the eager blob downloader may
9-
* try to fetch the same blob concurrently. This tracker ensures each
10-
* unique blob ref is only downloaded once — subsequent requests for
11-
* the same ref piggyback on the existing promise.
26+
* Both the blob-resolve middleware and the eager blob downloader use this
27+
* tracker. Instantiate once per DexieCloudDB.
28+
*/
29+
30+
/**
31+
* Maximum number of concurrent blob fetches.
1232
*
13-
* Instantiate once per DexieCloudDB.
33+
* Historically 6 to match the HTTP/1.1 same-origin connection cap that
34+
* browsers enforce. With HTTP/2 (the typical transport for Dexie Cloud
35+
* today) many streams multiplex over a single TCP connection, so the
36+
* old cap is overly conservative. 10 is a modest bump that still keeps
37+
* memory pressure (in-flight Uint8Arrays) and server load bounded.
38+
* Can be made configurable via DexieCloudOptions if a real need arises.
1439
*/
40+
export const MAX_CONCURRENT = 10;
41+
1542
export class BlobDownloadTracker {
1643
private inFlight = new Map<string, Promise<Uint8Array>>();
1744
private db: DexieCloudDB;
45+
private savingQueue: BlobSavingQueue;
46+
private activeFetches = 0;
47+
private waiting: Array<() => void> = [];
1848

1949
constructor(db: DexieCloudDB) {
2050
this.db = db;
51+
this.savingQueue = new BlobSavingQueue(db, (refs) => {
52+
// Called by the queue when a save transaction has completed
53+
// (regardless of success). Drop the in-flight cache entries now —
54+
// any future reader will go through IndexedDB instead.
55+
for (const ref of refs) {
56+
this.inFlight.delete(ref);
57+
}
58+
});
2159
}
2260

2361
/**
24-
* Download a blob, deduplicating concurrent requests for the same ref.
62+
* Download a blob, deduplicating concurrent requests for the same ref
63+
* and respecting the global fetch concurrency cap.
64+
*
65+
* Lifecycle:
66+
* - Slot is acquired before the fetch and released as soon as the
67+
* fetch settles (success or failure).
68+
* - The in-flight entry survives a successful fetch and lives on
69+
* until persistence completes (via enqueueSave) or releaseRefs
70+
* is called. On fetch failure, the entry is removed immediately
71+
* so a future call can retry.
2572
*
2673
* @param blobRef - The BlobRef to download
2774
* @param dbUrl - Base URL for the database (e.g., 'https://mydb.dexie.cloud')
2875
*/
2976
download(blobRef: BlobRef, dbUrl: string): Promise<Uint8Array> {
3077
let promise = this.inFlight.get(blobRef.ref);
3178
if (!promise) {
32-
promise = loadCachedAccessToken(this.db)
33-
.then((accessToken) => {
34-
// accessToken may be null for anonymous/unauthenticated users.
35-
// Public realm blobs (rlm-public) are accessible without auth.
36-
// downloadBlob will omit the Authorization header when token is null.
37-
return downloadBlob(blobRef, dbUrl, accessToken);
38-
})
39-
.finally(() => this.inFlight.delete(blobRef.ref));
40-
// When the promise settles (either fulfilled or rejected), remove it from the in-flight map
79+
promise = this.acquireSlot()
80+
.then(() =>
81+
this.downloadBlob(blobRef, dbUrl).finally(() => this.releaseSlot())
82+
)
83+
.catch((err) => {
84+
// On error, remove immediately so a future call can retry.
85+
// (Slot already released by the .finally above.)
86+
this.inFlight.delete(blobRef.ref);
87+
throw err;
88+
});
4189
this.inFlight.set(blobRef.ref, promise);
4290
}
4391
return promise;
4492
}
45-
}
46-
/**
47-
* Download blob data from server via proxy endpoint.
48-
* Uses auth header for authentication (same as sync).
49-
* When accessToken is null, the request is made without Authorization header —
50-
* this allows downloading blobs from public realms (rlm-public) for
51-
* unauthenticated users.
52-
*
53-
* @param blobRef - The BlobRef to download
54-
* @param dbUrl - Base URL for the database (e.g., 'https://mydb.dexie.cloud')
55-
* @param accessToken - Access token for authentication, or null for anonymous access
56-
*/
5793

58-
export async function downloadBlob(
59-
blobRef: BlobRef,
60-
dbUrl: string,
61-
accessToken: string | null
62-
): Promise<Uint8Array> {
63-
const downloadUrl = `${dbUrl}/blob/${blobRef.ref}`;
64-
const headers: HeadersInit = {};
65-
if (accessToken) {
66-
headers['Authorization'] = `Bearer ${accessToken}`;
94+
/**
95+
* Queue resolved blobs for persisting back to IndexedDB.
96+
* When the save transaction completes, the corresponding in-flight
97+
* entries are released.
98+
*/
99+
enqueueSave(
100+
tableName: string,
101+
primaryKey: any,
102+
resolvedBlobs: ResolvedBlob[]
103+
): void {
104+
this.savingQueue.saveBlobs(tableName, primaryKey, resolvedBlobs);
67105
}
68-
const response = await fetch(downloadUrl, { headers });
69106

70-
if (!response.ok) {
71-
throw new Error(
72-
`Failed to download blob ${blobRef.ref}: ${response.status} ${response.statusText}`
73-
);
107+
/**
108+
* Wait until all previously enqueued saves have been persisted to
109+
* IndexedDB. Used by callers that need to make decisions based on
110+
* on-disk state — e.g., the eager downloader looping over rows with
111+
* `_hasBlobRefs=1` in chunks, where each iteration must see the
112+
* previous chunk's writes before re-querying.
113+
*
114+
* New saves enqueued AFTER drainPendingSaves() is called do NOT extend
115+
* the wait.
116+
*/
117+
drainPendingSaves(): Promise<void> {
118+
return this.savingQueue.drain();
74119
}
75120

76-
const arrayBuffer = await response.arrayBuffer();
77-
return new Uint8Array(arrayBuffer);
121+
/**
122+
* Release in-flight entries without going through the internal saving
123+
* queue. Used when the caller persists the blobs itself, or when no
124+
* primary key was available and the data won't be persisted at all.
125+
*/
126+
releaseRefs(refs: string[]): void {
127+
for (const ref of refs) {
128+
this.inFlight.delete(ref);
129+
}
130+
}
131+
132+
private acquireSlot(): Promise<void> {
133+
if (this.activeFetches < MAX_CONCURRENT) {
134+
this.activeFetches++;
135+
return Promise.resolve();
136+
}
137+
return new Promise<void>((resolve) => {
138+
this.waiting.push(() => {
139+
this.activeFetches++;
140+
resolve();
141+
});
142+
});
143+
}
144+
145+
private releaseSlot(): void {
146+
this.activeFetches--;
147+
const next = this.waiting.shift();
148+
if (next) next();
149+
}
150+
151+
/**
152+
* Download blob data from server via proxy endpoint.
153+
* Uses auth header for authentication (same as sync).
154+
* When accessToken is null, the request is made without Authorization header —
155+
* this allows downloading blobs from public realms (rlm-public) for
156+
* unauthenticated users.
157+
*
158+
* @param blobRef - The BlobRef to download
159+
* @param dbUrl - Base URL for the database (e.g., 'https://mydb.dexie.cloud')
160+
*/
161+
162+
private async downloadBlob(
163+
blobRef: BlobRef,
164+
dbUrl: string
165+
): Promise<Uint8Array> {
166+
const accessToken = await loadCachedAccessToken(this.db);
167+
const downloadUrl = `${dbUrl}/blob/${blobRef.ref}`;
168+
const headers: HeadersInit = {};
169+
if (accessToken) {
170+
// accessToken may be null for anonymous/unauthenticated users.
171+
// Public realm blobs (rlm-public) are accessible without auth.
172+
// downloadBlob will omit the Authorization header when token is null.
173+
headers['Authorization'] = `Bearer ${accessToken}`;
174+
}
175+
// cache: 'no-store' prevents the browser from storing this response in its
176+
// HTTP cache. The server sets a long Expires/Cache-Control header on blob
177+
// responses (blobs are immutable and content-addressed), which would
178+
// otherwise cause the browser to keep a copy in its disk cache in addition
179+
// to the copy we persist to IndexedDB — doubling storage for every blob.
180+
// Since we always persist to IndexedDB and subsequent reads go through
181+
// IndexedDB (never re-fetch), the browser cache copy is pure overhead.
182+
const response = await fetch(downloadUrl, { headers, cache: 'no-store' });
183+
184+
if (!response.ok) {
185+
throw new Error(
186+
`Failed to download blob ${blobRef.ref}: ${response.status} ${response.statusText}`
187+
);
188+
}
189+
190+
const arrayBuffer = await response.arrayBuffer();
191+
return new Uint8Array(arrayBuffer);
192+
}
78193
}

0 commit comments

Comments
 (0)