Skip to content

Commit

Permalink
Resolve Issue 1765 (application hang if requireAuth: true in Dexie Cl…
Browse files Browse the repository at this point in the history
…oud) (#1766)

* Resolves #1765
The `db` passed to db.on.ready() callback must not block until db is ready because the callback's code is part of making the db ready. A bug prevented liveQueries based on the unblocked db to emit values before on-ready callback resolves its promise.

When using requireAuth: true, the entire authentication flow is done as a part of the on('ready') flow. A part of the flow was waiting for a liveQuery observable to emit currentUser value before continuing, leading to a deadlock.

There was a general bug that the "_vip" state of the db wasn't propagated to transactions and tables. This is solved by letting the "_vip" version of the db (the one passed to on-ready callback) be a Proxy that makes sure to return vipped tables and transactions so that all code using that db instance will not deadlock before the on-ready callback completes.

* Simplified arguments now that db._vip is a pure Proxy rather than a protype-derived object.

* When error occur, a consequential error happened: postMessage failed to post an object with function properties.
Make sure not to post the DexieError but a simpler one, copying the message only.

* Let the "ResetDatabaseButton" reload the page to ensure application state reset.

* One unit test fails. Must use Proxy instead of Object.create() for vipified tables / transactions.
This will makes uses less complex, especially places where code is setting props on the vipified objects without having to get adjusted for the vip case.

* Reverted unintentional code changes
  • Loading branch information
dfahlander authored Jul 16, 2023
1 parent bf147d3 commit e96dd29
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 13 deletions.
2 changes: 1 addition & 1 deletion addons/dexie-cloud/src/sync/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export function sync(
});
db.syncStateChangedEvent.next({
phase: isOnline ? 'error' : 'offline',
error,
error: new Error('' + error?.message || error),
});
return Promise.reject(error);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function ResetDatabaseButton() {
className="large-button"
onClick={async () => {
await db.delete();
await db.open();
location.reload(); // Reload the page to reset application state hard.
}}
>
<FontAwesomeIcon icon={faDatabase} /> Factory reset client
Expand Down
4 changes: 2 additions & 2 deletions src/classes/dexie/dexie-open.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ export function dexieOpen (db: Dexie) {
upgradeTransaction.onerror = eventRejectHandler(reject);
var oldVer = e.oldVersion > Math.pow(2, 62) ? 0 : e.oldVersion; // Safari 8 fix.
wasCreated = oldVer < 1;
db._novip.idbdb = req.result;// db._novip is because db can be an Object.create(origDb).
db.idbdb = req.result;
runUpgraders(db, oldVer / 10, upgradeTransaction, reject);
}
}, reject);

req.onsuccess = wrap (() => {
// Core opening procedure complete. Now let's just record some stuff.
upgradeTransaction = null;
const idbdb = db._novip.idbdb = req.result; // db._novip is because db can be an Object.create(origDb).
const idbdb = db.idbdb = req.result;

const objectStoreNames = slice(idbdb.objectStoreNames);
if (objectStoreNames.length > 0) try {
Expand Down
19 changes: 17 additions & 2 deletions src/classes/dexie/dexie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { IndexableType } from '../../public';
import { observabilityMiddleware } from '../../live-query/observability-middleware';
import { cacheExistingValuesMiddleware } from '../../dbcore/cache-existing-values-middleware';
import { cacheMiddleware } from "../../live-query/cache/cache-middleware";
import { vipify } from "../../helpers/vipify";

export interface DbReadyState {
dbOpenError: any;
Expand Down Expand Up @@ -220,7 +221,21 @@ export class Dexie implements IDexie {
this.use(virtualIndexMiddleware);
this.use(hooksMiddleware);

this.vip = Object.create(this, {_vip: {value: true}}) as Dexie;
const vipDB = new Proxy(this, {
get: (_, prop, receiver) => {
if (prop === '_vip') return true;
if (prop === 'table') return (tableName: string) => vipify(this.table(tableName), vipDB);
const rv = Reflect.get(_, prop, receiver);
if (rv instanceof Table) return vipify(rv, vipDB);
if (prop === 'tables') return (rv as Table[]).map(t => vipify(t, vipDB));
if (prop === '_createTransaction') return function() {
const tx: Transaction = (rv as typeof this._createTransaction).apply(this, arguments);
return vipify(tx, vipDB);
}
return rv;
}
});
this.vip = vipDB;

// Call each addon:
addons.forEach(addon => addon(this));
Expand Down Expand Up @@ -298,7 +313,7 @@ export class Dexie implements IDexie {
if (idx >= 0) connections.splice(idx, 1);
if (this.idbdb) {
try { this.idbdb.close(); } catch (e) { }
this._novip.idbdb = null; // db._novip is because db can be an Object.create(origDb).
this.idbdb = null;
}
// Reset dbReadyPromise promise:
state.dbReadyPromise = new Promise(resolve => {
Expand Down
2 changes: 1 addition & 1 deletion src/classes/dexie/generate-middleware-stacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function createMiddlewareStacks(
};
}

export function generateMiddlewareStacks({_novip: db}: Dexie, tmpTrans: IDBTransaction) {
export function generateMiddlewareStacks(db: Dexie, tmpTrans: IDBTransaction) {
const idbdb = tmpTrans.db;
const stacks = createMiddlewareStacks(db._middlewares, idbdb, db._deps, tmpTrans);
db.core = stacks.dbcore!;
Expand Down
11 changes: 5 additions & 6 deletions src/classes/version/schema-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import { exceptions } from '../../errors';
import { TableSchema } from '../../public/types/table-schema';
import { IndexSpec } from '../../public/types/index-spec';
import { hasIEDeleteObjectStoreBug, isIEOrEdge } from '../../globals/constants';
import { safariMultiStoreFix } from '../../functions/quirks';
import { createIndexSpec, nameFromKeyPath } from '../../helpers/index-spec';
import { createTableSchema } from '../../helpers/table-schema';
import { generateMiddlewareStacks } from '../dexie/generate-middleware-stacks';

export function setApiOnPlace({_novip: db}: Dexie, objs: Object[], tableNames: string[], dbschema: DbSchema) {
export function setApiOnPlace(db: Dexie, objs: Object[], tableNames: string[], dbschema: DbSchema) {
tableNames.forEach(tableName => {
const schema = dbschema[tableName];
objs.forEach(obj => {
Expand All @@ -41,7 +40,7 @@ export function setApiOnPlace({_novip: db}: Dexie, objs: Object[], tableNames: s
});
}

export function removeTablesApi({_novip: db}: Dexie, objs: Object[]) {
export function removeTablesApi(db: Dexie, objs: Object[]) {
objs.forEach(obj => {
for (let key in obj) {
if (obj[key] instanceof db.Table) delete obj[key];
Expand Down Expand Up @@ -78,7 +77,7 @@ export function runUpgraders(db: Dexie, oldVersion: number, idbUpgradeTrans: IDB
export type UpgradeQueueItem = (idbtrans: IDBTransaction) => PromiseLike<any> | void;

export function updateTablesAndIndexes(
{_novip: db}: Dexie,
db: Dexie,
oldVersion: number,
trans: Transaction,
idbUpgradeTrans: IDBTransaction)
Expand Down Expand Up @@ -339,7 +338,7 @@ function buildGlobalSchema(
return globalSchema;
}

export function readGlobalSchema({_novip: db}: Dexie, idbdb: IDBDatabase, tmpTrans: IDBTransaction) {
export function readGlobalSchema(db: Dexie, idbdb: IDBDatabase, tmpTrans: IDBTransaction) {
db.verno = idbdb.version / 10;
const globalSchema = db._dbSchema = buildGlobalSchema(db, idbdb, tmpTrans);
db._storeNames = slice(idbdb.objectStoreNames, 0);
Expand All @@ -352,7 +351,7 @@ export function verifyInstalledSchema(db: Dexie, tmpTrans: IDBTransaction): bool
return !(diff.add.length || diff.change.some(ch => ch.add.length || ch.change.length));
}

export function adjustToExistingIndexNames({_novip: db}: Dexie, schema: DbSchema, idbtrans: IDBTransaction) {
export function adjustToExistingIndexNames(db: Dexie, schema: DbSchema, idbtrans: IDBTransaction) {
// Issue #30 Problem with existing db - adjust to existing index names when migrating from non-dexie db
const storeNames = idbtrans.db.objectStoreNames;

Expand Down
18 changes: 18 additions & 0 deletions src/helpers/vipify.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { type Dexie } from "../classes/dexie";
import { type Table } from "../classes/table";
import { type Transaction } from "../classes/transaction";

export function vipify<T extends Table | Transaction>(
target: T,
vipDb: Dexie
): T {
return new Proxy(target, {
get (target, prop, receiver) {
// The "db" prop of the table or transaction is the only one we need to
// override. The rest of the props can be accessed from the original
// object.
if (prop === 'db') return vipDb;
return Reflect.get(target, prop, receiver);
}
});
}

0 comments on commit e96dd29

Please sign in to comment.