From be60b3d2253a667ec673332eb35a561fad8847d5 Mon Sep 17 00:00:00 2001 From: Josh Perez <60019601+josh-signal@users.noreply.github.com> Date: Thu, 4 May 2023 13:59:02 -0400 Subject: [PATCH] Moves SQL to full IPC --- app/main.ts | 1 - test/test.js | 11 -- ts/background.ts | 9 -- ts/services/networkObserver.ts | 7 - ts/sql/Client.ts | 259 ++++++--------------------------- ts/sql/Interface.ts | 9 -- ts/sql/Server.ts | 75 +--------- ts/sql/channels.ts | 61 ++++++++ ts/util/createIPCEvents.ts | 7 - ts/windows/main/start.ts | 4 +- ts/windows/preload.ts | 1 - 11 files changed, 110 insertions(+), 334 deletions(-) create mode 100644 ts/sql/channels.ts diff --git a/app/main.ts b/app/main.ts index 144594e51..7ab02d109 100644 --- a/app/main.ts +++ b/app/main.ts @@ -1529,7 +1529,6 @@ const onDatabaseError = async (error: string) => { ready = false; if (mainWindow) { - drop(settingsChannel?.invokeCallbackInMainWindow('closeDB', [])); mainWindow.close(); } mainWindow = undefined; diff --git a/test/test.js b/test/test.js index 3176eed07..f4f73f84d 100644 --- a/test/test.js +++ b/test/test.js @@ -23,17 +23,6 @@ before(async () => { window.testUtilities.installMessageController(); await deleteIndexedDB(); - try { - window.SignalContext.log.info('Initializing SQL in renderer'); - const isTesting = true; - await window.Signal.Data.startInRenderer(isTesting); - window.SignalContext.log.info('SQL initialized in renderer'); - } catch (err) { - window.SignalContext.log.error( - 'SQL failed to initialize', - err && err.stack ? err.stack : err - ); - } await window.testUtilities.initializeMessageCounter(); await window.Signal.Data.removeAll(); await window.storage.fetch(); diff --git a/ts/background.ts b/ts/background.ts index b22ebdbb7..d2fce6343 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -944,12 +944,6 @@ export async function startApp(): Promise { void window.Signal.Data.ensureFilePermissions(); } - try { - await window.Signal.Data.startInRendererProcess(); - } catch (err) { - log.error('SQL failed to initialize', Errors.toLogFormat(err)); - } - setAppLoadingScreenMessage(window.i18n('icu:loading'), window.i18n); let isMigrationWithIndexComplete = false; @@ -2590,9 +2584,6 @@ export async function startApp(): Promise { // Start listeners here, after we get through our queue. RotateSignedPreKeyListener.init(window.Whisper.events, newVersion); - // Go back to main process before processing delayed actions - await window.Signal.Data.goBackToMainProcess(); - profileKeyResponseQueue.start(); lightSessionResetQueue.start(); onDecryptionErrorQueue.start(); diff --git a/ts/services/networkObserver.ts b/ts/services/networkObserver.ts index 19a8c5723..8af7c1aa6 100644 --- a/ts/services/networkObserver.ts +++ b/ts/services/networkObserver.ts @@ -8,7 +8,6 @@ import type { import { getSocketStatus } from '../shims/socketStatus'; import * as log from '../logging/log'; import { SECOND } from '../util/durations'; -import { SocketStatus } from '../types/SocketStatus'; type NetworkActions = { checkNetworkStatus: (x: CheckNetworkStatusPayloadType) => NetworkActionType; @@ -23,12 +22,6 @@ export function initializeNetworkObserver( const refresh = () => { const socketStatus = getSocketStatus(); - if (socketStatus === SocketStatus.CLOSED) { - // If we couldn't connect during startup - we should still switch SQL to - // the main process to avoid stalling UI. - void window.Signal.Data.goBackToMainProcess(); - } - networkActions.checkNetworkStatus({ isOnline: navigator.onLine, socketStatus, diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index 0c05db1c0..96754f48d 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -2,29 +2,16 @@ // SPDX-License-Identifier: AGPL-3.0-only import { ipcRenderer as ipc } from 'electron'; -import fs from 'fs-extra'; -import pify from 'pify'; import PQueue from 'p-queue'; -import { - compact, - fromPairs, - groupBy, - isFunction, - isTypedArray, - last, - map, - omit, - toPairs, -} from 'lodash'; +import { has, get, groupBy, isTypedArray, last, map, omit } from 'lodash'; import { deleteExternalFiles } from '../types/Conversation'; import { expiringMessagesDeletionService } from '../services/expiringMessagesDeletion'; import { tapToViewMessagesDeletionService } from '../services/tapToViewMessagesDeletionService'; import * as Bytes from '../Bytes'; import { createBatcher } from '../util/batcher'; -import { explodePromise } from '../util/explodePromise'; -import { assertDev, softAssert, strictAssert } from '../util/assert'; +import { assertDev, softAssert } from '../util/assert'; import { mapObjectWithSpec } from '../util/mapObjectWithSpec'; import type { ObjectMappingSpecType } from '../util/mapObjectWithSpec'; import { cleanDataForIpc } from './cleanDataForIpc'; @@ -38,6 +25,7 @@ import type { StoredJob } from '../jobs/types'; import { formatJobForInsert } from '../jobs/formatJobForInsert'; import { cleanupMessage } from '../util/cleanup'; import { drop } from '../util/drop'; +import { ipcInvoke, doShutdown } from './channels'; import type { AdjacentMessagesByConversationOptionsType, @@ -65,18 +53,11 @@ import type { SignedPreKeyType, StoredSignedPreKeyType, } from './Interface'; -import Server from './Server'; -import { parseSqliteError, SqliteErrorKind } from './errors'; import { MINUTE } from '../util/durations'; import { getMessageIdForLogging } from '../util/idForLogging'; import type { MessageAttributesType } from '../model-types'; import { incrementMessageCounter } from '../util/incrementMessageCounter'; -const getRealPath = pify(fs.realpath); - -const MIN_TRACE_DURATION = 10; - -const SQL_CHANNEL_KEY = 'sql-channel'; const ERASE_SQL_KEY = 'erase-sql-key'; const ERASE_ATTACHMENTS_KEY = 'erase-attachments'; const ERASE_STICKERS_KEY = 'erase-stickers'; @@ -85,92 +66,6 @@ const ERASE_DRAFTS_KEY = 'erase-drafts'; const CLEANUP_ORPHANED_ATTACHMENTS_KEY = 'cleanup-orphaned-attachments'; const ENSURE_FILE_PERMISSIONS = 'ensure-file-permissions'; -enum RendererState { - InMain = 'InMain', - Opening = 'Opening', - InRenderer = 'InRenderer', - Closing = 'Closing', -} - -let activeJobCount = 0; -let resolveShutdown: (() => void) | undefined; -let shutdownPromise: Promise | null = null; - -let state = RendererState.InMain; -const startupQueries = new Map(); - -async function startInRendererProcess(isTesting = false): Promise { - strictAssert( - state === RendererState.InMain, - `startInRendererProcess: expected ${state} to be ${RendererState.InMain}` - ); - - log.info('data.startInRendererProcess: switching to renderer process'); - state = RendererState.Opening; - - if (!isTesting) { - await ipc.invoke('database-ready'); - } - - const configDir = await getRealPath(ipc.sendSync('get-user-data-path')); - const key = ipc.sendSync('user-config-key'); - - await Server.initializeRenderer({ configDir, key }); - - log.info('data.startInRendererProcess: switched to renderer process'); - - state = RendererState.InRenderer; -} - -async function goBackToMainProcess(): Promise { - if (state === RendererState.InMain) { - log.info('goBackToMainProcess: Already in the main process'); - return; - } - - strictAssert( - state === RendererState.InRenderer, - `goBackToMainProcess: expected ${state} to be ${RendererState.InRenderer}` - ); - - // We don't need to wait for pending queries since they are synchronous. - log.info('data.goBackToMainProcess: switching to main process'); - const closePromise = channels.close(); - - // It should be the last query we run in renderer process - state = RendererState.Closing; - await closePromise; - state = RendererState.InMain; - - // Print query statistics for whole startup - const entries = Array.from(startupQueries.entries()); - startupQueries.clear(); - - // Sort by decreasing duration - entries - .sort((a, b) => b[1] - a[1]) - .filter(([_, duration]) => duration > MIN_TRACE_DURATION) - .forEach(([query, duration]) => { - log.info(`startup query: ${query} ${duration}ms`); - }); - - log.info('data.goBackToMainProcess: switched to main process'); -} - -const channelsAsUnknown = fromPairs( - compact( - map(toPairs(Server), ([name, value]: [string, unknown]) => { - if (isFunction(value)) { - return [name, makeChannel(name)]; - } - - return null; - }) - ) -) as unknown; - -const channels: ServerInterface = channelsAsUnknown as ServerInterface; - const exclusiveInterface: ClientExclusiveInterface = { createOrUpdateIdentityKey, getIdentityKeyById, @@ -209,30 +104,53 @@ const exclusiveInterface: ClientExclusiveInterface = { removeOtherData, cleanupOrphanedAttachments, ensureFilePermissions, - - // Client-side only, and test-only - - startInRendererProcess, - goBackToMainProcess, }; -// Because we can't force this module to conform to an interface, we narrow our exports -// to this one default export, which does conform to the interface. -// Note: In Javascript, you need to access the .default property when requiring it -// https://github.com/microsoft/TypeScript/issues/420 -const dataInterface: ClientInterface = { - ...channels, - ...exclusiveInterface, +type ClientOverridesType = ClientExclusiveInterface & + Pick< + ServerInterface, + | 'removeMessage' + | 'removeMessages' + | 'saveAttachmentDownloadJob' + | 'saveMessage' + | 'saveMessages' + | 'updateConversations' + >; - // Overrides - updateConversations, - saveMessage, - saveMessages, +const channels: ServerInterface = new Proxy({} as ServerInterface, { + get(_target, name) { + return async (...args: ReadonlyArray) => + ipcInvoke(String(name), args); + }, +}); + +const clientExclusiveOverrides: ClientOverridesType = { + ...exclusiveInterface, removeMessage, removeMessages, saveAttachmentDownloadJob, + saveMessage, + saveMessages, + updateConversations, }; +const dataInterface: ClientInterface = new Proxy( + { + ...clientExclusiveOverrides, + } as ClientInterface, + { + get(target, name) { + return async (...args: ReadonlyArray) => { + if (has(target, name)) { + return get(target, name)(...args); + } + + return get(channels, name)(...args); + }; + }, + } +); + export default dataInterface; function _cleanData( @@ -272,99 +190,6 @@ export function _cleanMessageData(data: MessageType): MessageType { return _cleanData(omit(result, ['dataMessage'])); } -async function doShutdown() { - log.info( - `data.shutdown: shutdown requested. ${activeJobCount} jobs outstanding` - ); - - if (shutdownPromise) { - return shutdownPromise; - } - - // No outstanding jobs, return immediately - if (activeJobCount === 0) { - return; - } - - ({ promise: shutdownPromise, resolve: resolveShutdown } = - explodePromise()); - - try { - await shutdownPromise; - } finally { - log.info('data.shutdown: process complete'); - } -} - -function makeChannel(fnName: string) { - return async (...args: ReadonlyArray) => { - // During startup we want to avoid the high overhead of IPC so we utilize - // the db that exists in the renderer process to be able to boot up quickly - // once the app is running we switch back to the main process to avoid the - // UI from locking up whenever we do costly db operations. - if (state === RendererState.InRenderer) { - const serverFnName = fnName as keyof ServerInterface; - const serverFn = Server[serverFnName] as ( - ...fnArgs: ReadonlyArray - ) => unknown; - const start = Date.now(); - - try { - // Ignoring this error TS2556: Expected 3 arguments, but got 0 or more. - return await serverFn(...args); - } catch (error) { - const sqliteErrorKind = parseSqliteError(error); - if (sqliteErrorKind === SqliteErrorKind.Corrupted) { - log.error( - 'Detected sql corruption in renderer process. ' + - `Restarting the application immediately. Error: ${error.message}` - ); - ipc?.send('database-error', error.stack); - } else if (sqliteErrorKind === SqliteErrorKind.Readonly) { - log.error(`Detected readonly sql database: ${error.message}`); - ipc?.send('database-readonly'); - } - - log.error( - `Renderer SQL channel job (${fnName}) error ${error.message}` - ); - throw error; - } finally { - const duration = Date.now() - start; - - startupQueries.set( - serverFnName, - (startupQueries.get(serverFnName) || 0) + duration - ); - - if (duration > MIN_TRACE_DURATION) { - log.info( - `Renderer SQL channel job (${fnName}) completed in ${duration}ms` - ); - } - } - } - - if (shutdownPromise && fnName !== 'close') { - throw new Error( - `Rejecting SQL channel job (${fnName}); application is shutting down` - ); - } - - activeJobCount += 1; - return createTaskWithTimeout(async () => { - try { - return await ipc.invoke(SQL_CHANNEL_KEY, fnName, ...args); - } finally { - activeJobCount -= 1; - if (activeJobCount === 0) { - resolveShutdown?.(); - } - } - }, `SQL channel call (${fnName})`)(); - }; -} - function specToBytes( spec: ObjectMappingSpecType, data: Input diff --git a/ts/sql/Interface.ts b/ts/sql/Interface.ts index 8a052d413..2348b0056 100644 --- a/ts/sql/Interface.ts +++ b/ts/sql/Interface.ts @@ -830,10 +830,6 @@ export type ServerInterface = DataInterface & { key: string; logger: LoggerType; }) => Promise; - initializeRenderer: (options: { - configDir: string; - key: string; - }) => Promise; getKnownMessageAttachments: ( cursor?: MessageAttachmentsCursorType @@ -913,11 +909,6 @@ export type ClientExclusiveInterface = { removeOtherData: () => Promise; cleanupOrphanedAttachments: () => Promise; ensureFilePermissions: () => Promise; - - // To decide whether to use IPC to use the database in the main process or - // use the db already running in the renderer. - goBackToMainProcess: () => Promise; - startInRendererProcess: (isTesting?: boolean) => Promise; }; export type ClientInterface = DataInterface & ClientExclusiveInterface; diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index e39af10f1..019acb259 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -365,7 +365,6 @@ const dataInterface: ServerInterface = { // Server-only initialize, - initializeRenderer, getKnownMessageAttachments, finishGetKnownMessageAttachments, @@ -431,14 +430,6 @@ function rowToSticker(row: StickerRow): StickerType { }; } -function isRenderer() { - if (typeof process === 'undefined' || !process) { - return true; - } - - return process.type === 'renderer'; -} - function keyDatabase(db: Database, key: string): void { // https://www.zetetic.net/sqlcipher/sqlcipher-api/#key db.pragma(`key = "x'${key}'"`); @@ -522,7 +513,6 @@ function openAndSetUpSQLCipher(filePath: string, { key }: { key: string }) { let globalInstance: Database | undefined; let logger = consoleLogger; -let globalInstanceRenderer: Database | undefined; let databaseFilePath: string | undefined; let indexedDBPath: string | undefined; @@ -583,63 +573,13 @@ async function initialize({ } } -async function initializeRenderer({ - configDir, - key, -}: { - configDir: string; - key: string; -}): Promise { - if (!isRenderer()) { - throw new Error('Cannot call from main process.'); - } - if (globalInstanceRenderer) { - throw new Error('Cannot initialize more than once!'); - } - if (!isString(configDir)) { - throw new Error('initialize: configDir is required!'); - } - if (!isString(key)) { - throw new Error('initialize: key is required!'); - } - - if (!indexedDBPath) { - indexedDBPath = join(configDir, 'IndexedDB'); - } - - const dbDir = join(configDir, 'sql'); - - if (!databaseFilePath) { - databaseFilePath = join(dbDir, 'db.sqlite'); - } - - let promisified: Database | undefined; - - try { - promisified = openAndSetUpSQLCipher(databaseFilePath, { key }); - - // At this point we can allow general access to the database - globalInstanceRenderer = promisified; - - // test database - getMessageCountSync(); - } catch (error) { - log.error('Database startup error:', error.stack); - throw error; - } -} - async function close(): Promise { - for (const dbRef of [globalInstanceRenderer, globalInstance]) { - // SQLLite documentation suggests that we run `PRAGMA optimize` right - // before closing the database connection. - dbRef?.pragma('optimize'); - - dbRef?.close(); - } + // SQLLite documentation suggests that we run `PRAGMA optimize` right + // before closing the database connection. + globalInstance?.pragma('optimize'); + globalInstance?.close(); globalInstance = undefined; - globalInstanceRenderer = undefined; } async function removeDB(): Promise { @@ -676,13 +616,6 @@ async function removeIndexedDBFiles(): Promise { } function getInstance(): Database { - if (isRenderer()) { - if (!globalInstanceRenderer) { - throw new Error('getInstance: globalInstanceRenderer not set!'); - } - return globalInstanceRenderer; - } - if (!globalInstance) { throw new Error('getInstance: globalInstance not set!'); } diff --git a/ts/sql/channels.ts b/ts/sql/channels.ts new file mode 100644 index 000000000..1d25df125 --- /dev/null +++ b/ts/sql/channels.ts @@ -0,0 +1,61 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { ipcRenderer } from 'electron'; +import * as log from '../logging/log'; +import createTaskWithTimeout from '../textsecure/TaskWithTimeout'; +import { explodePromise } from '../util/explodePromise'; + +const SQL_CHANNEL_KEY = 'sql-channel'; +let activeJobCount = 0; +let resolveShutdown: (() => void) | undefined; +let shutdownPromise: Promise | null = null; + +export async function ipcInvoke( + name: string, + args: ReadonlyArray +): Promise { + const fnName = String(name); + + if (shutdownPromise && name !== 'close') { + throw new Error( + `Rejecting SQL channel job (${fnName}); application is shutting down` + ); + } + + activeJobCount += 1; + return createTaskWithTimeout(async () => { + try { + return await ipcRenderer.invoke(SQL_CHANNEL_KEY, name, ...args); + } finally { + activeJobCount -= 1; + if (activeJobCount === 0) { + resolveShutdown?.(); + } + } + }, `SQL channel call (${fnName})`)(); +} + +export async function doShutdown(): Promise { + log.info( + `data.shutdown: shutdown requested. ${activeJobCount} jobs outstanding` + ); + + if (shutdownPromise) { + return shutdownPromise; + } + + // No outstanding jobs, return immediately + if (activeJobCount === 0) { + return; + } + + ({ promise: shutdownPromise, resolve: resolveShutdown } = + explodePromise()); + + try { + await shutdownPromise; + } finally { + log.info('data.shutdown: process complete'); + } +} diff --git a/ts/util/createIPCEvents.ts b/ts/util/createIPCEvents.ts index d4ee70e29..c605e3321 100644 --- a/ts/util/createIPCEvents.ts +++ b/ts/util/createIPCEvents.ts @@ -102,7 +102,6 @@ export type IPCEventsCallbacksType = { authorizeArtCreator: (data: AuthorizeArtCreatorDataType) => void; deleteAllData: () => Promise; deleteAllMyStories: () => Promise; - closeDB: () => Promise; editCustomColor: (colorId: string, customColor: CustomColorType) => void; getConversationsWithCustomColor: (x: string) => Array; installStickerPack: (packId: string, key: string) => Promise; @@ -477,15 +476,9 @@ export function createIPCEvents( window.reduxActions.globalModals.showShortcutGuideModal(), deleteAllData: async () => { - await window.Signal.Data.goBackToMainProcess(); - renderClearingDataView(); }, - closeDB: async () => { - await window.Signal.Data.goBackToMainProcess(); - }, - showStickerPack: (packId, key) => { // We can get these events even if the user has never linked this instance. if (!Registration.everDone()) { diff --git a/ts/windows/main/start.ts b/ts/windows/main/start.ts index 3a079962c..86534d098 100644 --- a/ts/windows/main/start.ts +++ b/ts/windows/main/start.ts @@ -20,6 +20,7 @@ import { start as startConversationController } from '../../ConversationControll import { MessageController } from '../../util/MessageController'; import { Environment, getEnvironment } from '../../environment'; import { isProduction } from '../../util/version'; +import { ipcInvoke } from '../../sql/channels'; window.addEventListener('contextmenu', e => { const node = e.target as Element | null; @@ -46,7 +47,6 @@ startConversationController(); if (!isProduction(window.SignalContext.getVersion())) { const SignalDebug = { - Data: window.Signal.Data, cdsLookup: (options: CdsLookupOptionsType) => window.textsecure.server?.cdsLookup(options), getConversation: (id: string) => window.ConversationController.get(id), @@ -67,6 +67,8 @@ if (!isProduction(window.SignalContext.getVersion())) { setSfuUrl: (url: string) => { window.Signal.Services.calling._sfuUrl = url; }, + sqlCall: (name: string, ...args: ReadonlyArray) => + ipcInvoke(name, args), }; contextBridge.exposeInMainWorld('SignalDebug', SignalDebug); diff --git a/ts/windows/preload.ts b/ts/windows/preload.ts index 38cba13c2..363f767f6 100644 --- a/ts/windows/preload.ts +++ b/ts/windows/preload.ts @@ -15,7 +15,6 @@ installCallback('resetDefaultChatColor'); installCallback('setGlobalDefaultConversationColor'); installCallback('getDefaultConversationColor'); installCallback('persistZoomFactor'); -installCallback('closeDB'); // Getters only. These are set by the primary device installSetting('blockedCount', {