GitLab

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.

@@ -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 => {

@@ -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() {

@@ -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();

@@ -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.
+    *

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

Commits:

4 changed files:

Changes: