Pier Angelo Vendrame pushed to branch tor-browser-128.6.0esr-14.5-1 at The Tor Project / Applications / Tor Browser

Commits: 4b7dc6cc by Henry Wilkes at 2025-01-21T15:54:08+00:00 fixup! TB 40597: Implement TorSettings module

TB 41921: Do not wait for TorSettings to initialise before allowing a bootstrap.

Instead, we wait for TorSettings as required: for preparing AutoBootstrap attempts, and TorBootstrapRequest should already wait for TorSettings before attempting to connect.

- - - - - 34cfa54a by Henry Wilkes at 2025-01-21T15:54:13+00:00 fixup! TB 40933: Add tor-launcher functionality

TB 41921: Ensure TorProviderBuilder.build can be called immediately after TorProviderBuilder.init returns (without await).

We make sure that the `#provider` instance is created before any break in execution caused by an `await`.

- - - - - 1dc47dd9 by Henry Wilkes at 2025-01-21T15:54:14+00:00 fixup! TB 42247: Android helpers for the TorProvider

TB 41921: Do not wait for TorProviderBuilder.init since it is no longer async.

- - - - - fff7daea by Henry Wilkes at 2025-01-21T15:54:14+00:00 fixup! TB 40933: Add tor-launcher functionality

TB 41921: Add a note to TorBootstrapRequest explaining the expectation that the TorProvider has already read from TorSettings before it connects.

- - - - -

4 changed files:

- toolkit/components/tor-launcher/TorBootstrapRequest.sys.mjs - toolkit/components/tor-launcher/TorProviderBuilder.sys.mjs - toolkit/modules/TorAndroidIntegration.sys.mjs - toolkit/modules/TorConnect.sys.mjs

Changes:

===================================== toolkit/components/tor-launcher/TorBootstrapRequest.sys.mjs ===================================== @@ -90,6 +90,11 @@ export class TorBootstrapRequest { // Wait for bootstrapping to begin and maybe handle error. // Notice that we do not resolve the promise here in case of success, but // we do it from the BootstrapStatus observer. + // NOTE: After TorProviderBuilder.build resolves, TorProvider.init will + // have completed. In particular, assuming no errors, the TorSettings will + // have been initialised and passed on to the provider via + // TorProvider.writeSettings. Therefore we should be safe to immediately + // call `connect` using the latest user settings. lazy.TorProviderBuilder.build() .then(provider => provider.connect()) .catch(err => {

===================================== toolkit/components/tor-launcher/TorProviderBuilder.sys.mjs ===================================== @@ -54,14 +54,15 @@ export class TorProviderBuilder {

/** * Initialize the provider of choice. - * Even though initialization is asynchronous, we do not expect the caller to - * await this method. The reason is that any call to build() will wait the - * initialization anyway (and re-throw any initialization error). */ - static async init() { + static init() { switch (this.providerType) { case TorProviders.tor: - await this.#initTorProvider(); + // Even though initialization of the initial TorProvider is + // asynchronous, we do not expect the caller to await it. The reason is + // that any call to build() will wait the initialization anyway (and + // re-throw any initialization error). + this.#initTorProvider(); break; case TorProviders.none: lazy.TorLauncherUtil.setProxyConfiguration( @@ -74,7 +75,12 @@ export class TorProviderBuilder { } }

- static async #initTorProvider() { + /** + * Replace #provider with a new instance. + * + * @returns {Promise<TorProvider>} The new instance. + */ + static #initTorProvider() { if (!this.#exitObserver) { this.#exitObserver = this.#torExited.bind(this); Services.obs.addObserver( @@ -83,18 +89,39 @@ export class TorProviderBuilder { ); }

+ // NOTE: We need to ensure that the #provider is set as soon + // TorProviderBuilder.init is called. + // I.e. it should be safe to call + // TorProviderBuilder.init(); + // TorProviderBuilder.build(); + // without any await. + // + // Therefore, we await the oldProvider within the Promise rather than make + // #initTorProvider async. + // + // In particular, this is needed by TorConnect when the user has selected + // quickstart, in which case `TorConnect.init` will immediately request the + // provider. See tor-browser#41921. + this.#provider = this.#replaceTorProvider(this.#provider); + return this.#provider; + } + + /** + * Replace a TorProvider instance. Resolves once the TorProvider is + * initialised. + * + * @param {Promise<TorProvider>?} oldProvider - The previous's provider's + * promise, if any. + * @returns {TorProvider} The new TorProvider instance. + */ + static async #replaceTorProvider(oldProvider) { try { - const old = await this.#provider; - old?.uninit(); + // Uninitialise the old TorProvider, if there is any. + (await oldProvider)?.uninit(); } catch {} - this.#provider = new Promise((resolve, reject) => { - const provider = new lazy.TorProvider(); - provider - .init() - .then(() => resolve(provider)) - .catch(reject); - }); - await this.#provider; + const provider = new lazy.TorProvider(); + await provider.init(); + return provider; }

static uninit() {

===================================== toolkit/modules/TorAndroidIntegration.sys.mjs ===================================== @@ -68,9 +68,12 @@ class TorAndroidIntegrationImpl { Services.obs.addObserver(this, lazy.TorSettingsTopics[topic]); }

- lazy.TorProviderBuilder.init().finally(() => { - lazy.TorProviderBuilder.firstWindowLoaded(); - }); + lazy.TorProviderBuilder.init(); + // On Android immediately call firstWindowLoaded. This should be safe to + // call since it will await the initialisation of the TorProvider set up + // by TorProviderBuilder.init. + lazy.TorProviderBuilder.firstWindowLoaded(); + try { await lazy.TorSettings.init(); await lazy.TorConnect.init();

===================================== toolkit/modules/TorConnect.sys.mjs ===================================== @@ -10,7 +10,6 @@ ChromeUtils.defineESModuleGetters(lazy, { BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.sys.mjs", MoatRPC: "resource://gre/modules/Moat.sys.mjs", TorBootstrapRequest: "resource://gre/modules/TorBootstrapRequest.sys.mjs", - TorProviderBuilder: "resource://gre/modules/TorProviderBuilder.sys.mjs", TorProviderTopics: "resource://gre/modules/TorProviderBuilder.sys.mjs", TorLauncherUtil: "resource://gre/modules/TorLauncherUtil.sys.mjs", TorSettings: "resource://gre/modules/TorSettings.sys.mjs", @@ -436,15 +435,23 @@ class AutoBootstrapAttempt { return; } this.#resolved = true; - try { - // Run cleanup before we resolve the promise to ensure two instances - // of AutoBootstrapAttempt are not trying to change the settings at - // the same time. - if (arg.result !== "complete") { - await lazy.TorSettings.clearTemporaryBridges(); + + if (lazy.TorSettings.initialized) { + // If not initialized, then we won't have applied any settings. + try { + // Run cleanup before we resolve the promise to ensure two instances + // of AutoBootstrapAttempt are not trying to change the settings at + // the same time. + if (arg.result !== "complete") { + // Should do nothing if we never called applyTemporaryBridges. + await lazy.TorSettings.clearTemporaryBridges(); + } + } catch (error) { + lazy.logger.error( + "Unexpected error in auto-bootstrap cleanup", + error + ); } - } catch (error) { - lazy.logger.error("Unexpected error in auto-bootstrap cleanup", error); } if (arg.error) { reject(arg.error); @@ -483,6 +490,16 @@ class AutoBootstrapAttempt { * @param {BootstrapOptions} options - Options to apply to the bootstrap. */ async #runInternal(progressCallback, options) { + // Wait for TorSettings to be initialised, which is used for the + // AutoBootstrapping set up. + await Promise.race([ + lazy.TorSettings.initializedPromise, + this.#cancelledPromise, + ]); + if (this.#cancelled || this.#resolved) { + return; + } + await this.#fetchBridges(options); if (this.#cancelled || this.#resolved) { return; @@ -1016,19 +1033,20 @@ export const TorConnect = { lazy.logger.debug(`Observing topic '${addTopic}'`); };

- // Wait for TorSettings, as we will need it. - // We will wait for a TorProvider only after TorSettings is ready, - // because the TorProviderBuilder initialization might not have finished - // at this point, and TorSettings initialization is a prerequisite for - // having a provider. - // So, we prefer initializing TorConnect as soon as possible, so that - // the UI will be able to detect it is in the Initializing state and act - // consequently. - lazy.TorSettings.initializedPromise.then(() => this._settingsInitialized()); - // register the Tor topics we always care about observeTopic(lazy.TorProviderTopics.ProcessExited); observeTopic(lazy.TorProviderTopics.HasWarnOrErr); + + // NOTE: At this point, _requestedStage should still be `null`. + this._setStage(TorConnectStage.Start); + if ( + // Quickstart setting is enabled. + this.quickstart && + // And the previous bootstrap attempt must have succeeded. + !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) + ) { + this.beginBootstrapping(); + } },

async observe(subject, topic) { @@ -1058,26 +1076,6 @@ export const TorConnect = { } },

- async _settingsInitialized() { - // TODO: Handle failures here, instead of the prompt to restart the - // daemon when it exits (tor-browser#21053, tor-browser#41921). - await lazy.TorProviderBuilder.build(); - - lazy.logger.debug("The TorProvider is ready, changing state."); - // NOTE: If the tor process exits before this point, then - // shouldQuickStart would be `false`. - // NOTE: At this point, _requestedStage should still be `null`. - this._setStage(TorConnectStage.Start); - if ( - // Quickstart setting is enabled. - this.quickstart && - // And the previous bootstrap attempt must have succeeded. - !Services.prefs.getBoolPref(TorConnectPrefs.prompt_at_startup, true) - ) { - this.beginBootstrapping(); - } - }, - /** * Set the user stage. *

View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/b5cd602...