From a167b0a9d5a628b713855597c10332fed5227248 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Mon, 12 Aug 2024 15:17:29 -0500 Subject: [PATCH] Separate calls in sql channel Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> --- app/sql_channel.ts | 14 +++++++++++- ts/sql/Client.ts | 3 ++- ts/sql/Interface.ts | 3 +-- ts/sql/Server.ts | 53 ++++++++++++++++++++++++++------------------ ts/sql/channels.ts | 5 +++++ ts/sql/main.ts | 16 ++++++++----- ts/sql/mainWorker.ts | 41 +++++++++++++++++++++------------- 7 files changed, 90 insertions(+), 45 deletions(-) diff --git a/app/sql_channel.ts b/app/sql_channel.ts index 8911e1c6ad..57415e0c23 100644 --- a/app/sql_channel.ts +++ b/app/sql_channel.ts @@ -10,7 +10,11 @@ import { remove as removeEphemeralConfig } from './ephemeral_config'; let sql: | Pick< MainSQL, - 'sqlRead' | 'sqlWrite' | 'pauseWriteAccess' | 'resumeWriteAccess' + | 'sqlRead' + | 'sqlWrite' + | 'pauseWriteAccess' + | 'resumeWriteAccess' + | 'removeDB' > | undefined; @@ -18,6 +22,7 @@ let initialized = false; const SQL_READ_KEY = 'sql-channel:read'; const SQL_WRITE_KEY = 'sql-channel:write'; +const SQL_REMOVE_DB_KEY = 'sql-channel:remove-db'; const ERASE_SQL_KEY = 'erase-sql-key'; const PAUSE_WRITE_ACCESS = 'pause-sql-writes'; const RESUME_WRITE_ACCESS = 'resume-sql-writes'; @@ -44,6 +49,13 @@ export function initialize(mainSQL: typeof sql): void { return sql.sqlWrite(callName, ...args); }); + ipcMain.handle(SQL_REMOVE_DB_KEY, () => { + if (!sql) { + throw new Error(`${SQL_REMOVE_DB_KEY}: Not yet initialized!`); + } + return sql.removeDB(); + }); + ipcMain.handle(ERASE_SQL_KEY, () => { removeUserConfig(); removeEphemeralConfig(); diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index 1b979ca9ad..34b81a9d08 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -24,7 +24,7 @@ import * as Errors from '../types/errors'; import type { StoredJob } from '../jobs/types'; import { formatJobForInsert } from '../jobs/formatJobForInsert'; import { cleanupMessages } from '../util/cleanup'; -import { AccessType, ipcInvoke, doShutdown } from './channels'; +import { AccessType, ipcInvoke, doShutdown, removeDB } from './channels'; import type { ClientInterfaceWrap, @@ -124,6 +124,7 @@ const clientOnlyWritable: ClientOnlyWritableInterface = { flushUpdateConversationBatcher, + removeDB, shutdown, removeMessagesInConversation, diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index c82857bb77..04732060f3 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -646,8 +646,6 @@ type ReadableInterface = { type WritableInterface = { close: () => void; - removeDB: () => void; - removeIndexedDBFiles: () => void; removeIdentityKeyById: (id: IdentityKeyIdType) => number; @@ -1118,6 +1116,7 @@ export type ClientOnlyWritableInterface = ClientInterfaceWrap<{ // Client-side only shutdown: () => void; + removeDB: () => void; removeMessagesInConversation: ( conversationId: string, options: { diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index 8fb11ed122..dace9a7e85 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -350,7 +350,6 @@ export const DataReader: ServerReadableInterface = { export const DataWriter: ServerWritableInterface = { close: closeWritable, - removeDB, removeIndexedDBFiles, createOrUpdateIdentityKey, @@ -629,20 +628,29 @@ function openAndMigrateDatabase( // If that fails, we try to open the database with 3.x compatibility to extract the // user_version (previously stored in schema_version, blown away by cipher_migrate). db = new SQL(filePath) as WritableDB; - keyDatabase(db, key); + try { + keyDatabase(db, key); - // https://www.zetetic.net/blog/2018/11/30/sqlcipher-400-release/#compatability-sqlcipher-4-0-0 - db.pragma('cipher_compatibility = 3'); - migrateSchemaVersion(db); - db.close(); + // https://www.zetetic.net/blog/2018/11/30/sqlcipher-400-release/#compatability-sqlcipher-4-0-0 + db.pragma('cipher_compatibility = 3'); + migrateSchemaVersion(db); + db.close(); - // After migrating user_version -> schema_version, we reopen database, because we can't - // migrate to the latest ciphers after we've modified the defaults. - db = new SQL(filePath) as WritableDB; - keyDatabase(db, key); + // After migrating user_version -> schema_version, we reopen database, because + // we can't migrate to the latest ciphers after we've modified the defaults. + db = new SQL(filePath) as WritableDB; + keyDatabase(db, key); - db.pragma('cipher_migrate'); - switchToWAL(db); + db.pragma('cipher_migrate'); + switchToWAL(db); + } catch (error) { + try { + db.close(); + } catch { + // Best effort + } + throw error; + } return db; } @@ -659,8 +667,17 @@ function openAndSetUpSQLCipher( const db = openAndMigrateDatabase(filePath, key, readonly); - // Because foreign key support is not enabled by default! - db.pragma('foreign_keys = ON'); + try { + // Because foreign key support is not enabled by default! + db.pragma('foreign_keys = ON'); + } catch (error) { + try { + db.close(); + } catch { + // Best effort + } + throw error; + } return db; } @@ -750,13 +767,7 @@ function closeWritable(db: WritableDB): void { db.close(); } -function removeDB(db: WritableDB): void { - try { - db.close(); - } catch (error) { - logger.error('removeDB: Failed to close database:', error.stack); - } - +export function removeDB(): void { if (!databaseFilePath) { throw new Error( 'removeDB: Cannot erase database without a databaseFilePath!' diff --git a/ts/sql/channels.ts b/ts/sql/channels.ts index 56d26d8d4a..f360868ff5 100644 --- a/ts/sql/channels.ts +++ b/ts/sql/channels.ts @@ -9,6 +9,7 @@ import { missingCaseError } from '../util/missingCaseError'; const SQL_READ_KEY = 'sql-channel:read'; const SQL_WRITE_KEY = 'sql-channel:write'; +const SQL_REMOVE_DB_KEY = 'sql-channel:remove-db'; let activeJobCount = 0; let resolveShutdown: (() => void) | undefined; let shutdownPromise: Promise | null = null; @@ -77,3 +78,7 @@ export async function doShutdown(): Promise { log.info('data.shutdown: process complete'); } } + +export async function removeDB(): Promise { + return ipcRenderer.invoke(SQL_REMOVE_DB_KEY); +} diff --git a/ts/sql/main.ts b/ts/sql/main.ts index 85bfa579ef..33de0ecc84 100644 --- a/ts/sql/main.ts +++ b/ts/sql/main.ts @@ -9,6 +9,7 @@ import { app } from 'electron'; import { strictAssert } from '../util/assert'; import { explodePromise } from '../util/explodePromise'; import type { LoggerType } from '../types/Logging'; +import * as Errors from '../types/errors'; import { SqliteErrorKind } from './errors'; import type { ServerReadableDirectInterface, @@ -200,6 +201,16 @@ export class MainSQL { } public async close(): Promise { + if (this.onReady) { + try { + await this.onReady; + } catch (err) { + this.logger?.error(`MainSQL close, failed: ${Errors.toLogFormat(err)}`); + // Init failed + return; + } + } + if (!this.isReady) { throw new Error('Not initialized'); } @@ -256,11 +267,6 @@ export class MainSQL { await promise; } - // Special case since we need to broadcast this to every pool entry. - if (method === 'removeDB') { - return (await this.removeDB()) as Result; - } - const primary = this.pool[0]; const { result, duration } = await this.send(primary, { diff --git a/ts/sql/mainWorker.ts b/ts/sql/mainWorker.ts index ec0180533c..4d274fde64 100644 --- a/ts/sql/mainWorker.ts +++ b/ts/sql/mainWorker.ts @@ -11,7 +11,7 @@ import type { WrappedWorkerLogEntry, } from './main'; import type { WritableDB } from './Interface'; -import { initialize, DataReader, DataWriter } from './Server'; +import { initialize, DataReader, DataWriter, removeDB } from './Server'; import { SqliteErrorKind, parseSqliteError } from './errors'; if (!parentPort) { @@ -102,6 +102,31 @@ port.on('message', ({ seq, request }: WrappedWorkerRequest) => { return; } + // Removing database does not require active connection. + if (request.type === 'removeDB') { + try { + if (db) { + if (isPrimary) { + DataWriter.close(db); + } else { + DataReader.close(db); + } + db = undefined; + } + } catch (error) { + logger.error('Failed to close database before removal'); + } + + if (isPrimary) { + removeDB(); + } + + isRemoved = true; + + respond(seq, undefined, undefined); + return; + } + if (!db) { throw new Error('Not initialized'); } @@ -119,20 +144,6 @@ port.on('message', ({ seq, request }: WrappedWorkerRequest) => { return; } - if (request.type === 'removeDB') { - if (isPrimary) { - DataWriter.removeDB(db); - } else { - DataReader.close(db); - } - - isRemoved = true; - db = undefined; - - respond(seq, undefined, undefined); - return; - } - if (request.type === 'sqlCall:read' || request.type === 'sqlCall:write') { const DataInterface = request.type === 'sqlCall:read' ? DataReader : DataWriter;