From 3fe63c328f1dbbfa3092e9aed5f8fe94838684d5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 9 Nov 2022 14:34:40 +0100 Subject: [PATCH 1/4] fix(indexeddb): Call `IDBDatabase.close` manually. Surprisingly, `indexed_db_futures::IdbDatabase` is not closed when dropped. Hopefully, there is a [`IdbDatabase::close(&self)`][close] method, which calls `web_sys::IdbDatabase.close`, aka [`IDBDatabase.close`][websys-close], so let's use it! `IDBDatabase.close` returns immediately and closes the connection in a separate thread. The connection isn't actually closed until all transactions created using this connection are complete. No new transactions can be created for this connection once this method is called. Methods that create transactions throw an exception if a closing operation is pending. [close]: https://github.com/Alorel/rust-indexed-db/blob/8c106eb418aecdba2f3fde80d91a4673a875fdf6/src/idb_database.rs#L73-L77 [websys-close]: https://developer.mozilla.org/en-US/docs/Web/API/IDBDatabase/close --- crates/matrix-sdk-indexeddb/src/crypto_store.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store.rs b/crates/matrix-sdk-indexeddb/src/crypto_store.rs index fbd0caf58..7d88aa659 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store.rs @@ -283,6 +283,10 @@ impl IndexeddbCryptoStore { } }; + // Must release the database access manually as it's not done when + // dropping it. + db.close(); + IndexeddbCryptoStore::open_with_store_cipher(prefix, Some(store_cipher.into())).await } @@ -943,6 +947,14 @@ impl IndexeddbCryptoStore { } } +impl Drop for IndexeddbCryptoStore { + fn drop(&mut self) { + // Must release the database access manually as it's not done when + // dropping it. + self.inner.close(); + } +} + #[cfg(target_arch = "wasm32")] #[async_trait(?Send)] impl CryptoStore for IndexeddbCryptoStore { From f6496d01c78b492f29ac393d34327d3d4aff82b8 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 9 Nov 2022 14:38:16 +0100 Subject: [PATCH 2/4] feat(crypto-js): Add `OlmMachine.close`. This new `OlmMachine.close` forces to drop/close the `OlmMachine` without waiting on the JavaScript garbage collector to collect it. `wasm-bindgen` generates the following JS glue code: ```js close() { const ptr = this.__destroy_into_raw(); wasm.olmachine_close(ptr); } ``` And, `__destroy_into_raw` looks like this: ```js __destroy_into_raw() { const ptr = this.ptr; this.ptr = 0; OlmMachineFinalization.unregister(this); return ptr; } ``` It unregisters itself from the `FinalizationRegistry` correctly. We are protected from a double-free. --- bindings/matrix-sdk-crypto-js/src/machine.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bindings/matrix-sdk-crypto-js/src/machine.rs b/bindings/matrix-sdk-crypto-js/src/machine.rs index 84bd5befb..e2c140859 100644 --- a/bindings/matrix-sdk-crypto-js/src/machine.rs +++ b/bindings/matrix-sdk-crypto-js/src/machine.rs @@ -755,4 +755,7 @@ impl OlmMachine { passphrase, )?)?) } + + /// Force to drop/close the `OlmMachine`. + pub fn close(self) {} } From 196dfaea0308502f1ba70787960fa00cd0f6cb27 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 9 Nov 2022 14:42:23 +0100 Subject: [PATCH 3/4] test(crypto-js): Test `OlmMachine.close` + IndexedDB clean up. --- .../tests/machine.test.js | 53 ++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/bindings/matrix-sdk-crypto-js/tests/machine.test.js b/bindings/matrix-sdk-crypto-js/tests/machine.test.js index 4509f1515..e80b61f1d 100644 --- a/bindings/matrix-sdk-crypto-js/tests/machine.test.js +++ b/bindings/matrix-sdk-crypto-js/tests/machine.test.js @@ -29,17 +29,19 @@ describe(OlmMachine.name, () => { }); test('can be instantiated with a store', async () => { - // No databases. - expect(await indexedDB.databases()).toHaveLength(0); - let store_name = 'hello'; let store_passphrase = 'world'; + const by_store_name = db => db.name.startsWith(store_name); + + // No databases. + expect((await indexedDB.databases()).filter(by_store_name)).toHaveLength(0); + // Creating a new Olm machine. expect(await OlmMachine.initialize(new UserId('@foo:bar.org'), new DeviceId('baz'), store_name, store_passphrase)).toBeInstanceOf(OlmMachine); // Oh, there is 2 databases now, prefixed by `store_name`. - let databases = await indexedDB.databases(); + let databases = (await indexedDB.databases()).filter(by_store_name); expect(databases).toHaveLength(2); expect(databases).toStrictEqual([ @@ -51,7 +53,7 @@ describe(OlmMachine.name, () => { expect(await OlmMachine.initialize(new UserId('@foo:bar.org'), new DeviceId('baz'), store_name, store_passphrase)).toBeInstanceOf(OlmMachine); // Same number of databases. - expect(await indexedDB.databases()).toHaveLength(2); + expect((await indexedDB.databases()).filter(by_store_name)).toHaveLength(2); }); describe('cannot be instantiated with a store', () => { @@ -94,6 +96,45 @@ describe(OlmMachine.name, () => { return OlmMachine.initialize(new_user || user, new_device || device); } + test('can drop/close', async () => { + m = await machine(); + m.close(); + }) + + test('can drop/close with a store', async () => { + let store_name = 'temporary'; + let store_passphrase = 'temporary'; + + const by_store_name = db => db.name.startsWith(store_name); + + // No databases. + expect((await indexedDB.databases()).filter(by_store_name)).toHaveLength(0); + + // Creating a new Olm machine. + const m = await OlmMachine.initialize(new UserId('@foo:bar.org'), new DeviceId('baz'), store_name, store_passphrase); + expect(m).toBeInstanceOf(OlmMachine); + + // Oh, there is 2 databases now, prefixed by `store_name`. + let databases = (await indexedDB.databases()).filter(by_store_name); + + expect(databases).toHaveLength(2); + expect(databases).toStrictEqual([ + { name: `${store_name}::matrix-sdk-crypto-meta`, version: 1 }, + { name: `${store_name}::matrix-sdk-crypto`, version: 1 }, + ]); + + // Let's force to close the `OlmMachine`. + m.close(); + + // Now we can delete the databases! + for (const database_name of [`${store_name}::matrix-sdk-crypto`, `${store_name}::matrix-sdk-crypto-meta`]) { + const deleting = indexedDB.deleteDatabase(database_name); + deleting.onsuccess = () => {}; + deleting.onerror = () => { throw new Error('failed to remove the database (error)') }; + deleting.onblocked = () => { throw new Error('failed to remove the database (blocked)') }; + } + }) + test('can read user ID', async () => { expect((await machine()).userId.toString()).toStrictEqual(user.toString()); }); @@ -256,7 +297,7 @@ describe(OlmMachine.name, () => { beforeAll(async () => { m = await machine(user, device); }); - + test('can pass keysquery and keysclaim requests directly', async () => { { // derived from https://github.com/matrix-org/matrix-rust-sdk/blob/7f49618d350fab66b7e1dc4eaf64ec25ceafd658/benchmarks/benches/crypto_bench/keys_query.json From 6754defb9bd8170394a77d35444afb66653f65fe Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 10 Nov 2022 10:21:48 +0100 Subject: [PATCH 4/4] doc(crypto-js): Improve documentation of `OlmMachine.close`. --- bindings/matrix-sdk-crypto-js/src/machine.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bindings/matrix-sdk-crypto-js/src/machine.rs b/bindings/matrix-sdk-crypto-js/src/machine.rs index e2c140859..7d00cf179 100644 --- a/bindings/matrix-sdk-crypto-js/src/machine.rs +++ b/bindings/matrix-sdk-crypto-js/src/machine.rs @@ -756,6 +756,11 @@ impl OlmMachine { )?)?) } - /// Force to drop/close the `OlmMachine`. + /// Shut down the `OlmMachine`. + /// + /// The `OlmMachine` cannot be used after this method has been called. + /// + /// All associated resources will be closed too, like IndexedDB + /// connections. pub fn close(self) {} }