Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

setupTor should be called only once; polling for tor restart is broken #14584

Closed
riastradh-brave opened this issue Jun 27, 2018 · 7 comments
Closed

Comments

@riastradh-brave
Copy link
Contributor

riastradh-brave commented Jun 27, 2018

Test Plan

#14585

setupTor creates some resources (like a file system watcher) that there is no logic to destroy, and there should be no need to destroy in order to restart tor, but currently polling for tor restart is broken, so as a stop-gap measure, #14568 was written to call setupTor each time you ask to restart the tor daemon.

The polling logic should be fixed, obviating the need to call setupTor, and the resource leak arising from setupTor should be eliminated by not calling setupTor.

@riastradh-brave riastradh-brave self-assigned this Jun 27, 2018
riastradh-brave added a commit that referenced this issue Jun 27, 2018
Previously, we would never retry polling for tor launch if we ever
made the decision to open a control connection.

1. If we _haven't_ successfully opened a control connection, make
   sure we call this._polled() on all error paths to process a
   deferred file system watch notification.

2. If we have opened a control connection enough to set the `close`
   event handler, call this._polled() to handle a deferred poll
   (which, in the next tick, will either do the work it needs to do,
   or discover that there is already a control connection and do
   nothing).

3. In the `close` event handler for the control connection, poll for
   tor launch in case tor relaunched, and all the watch events were
   received and ignored, before we noticed that the control
   connection had closed.

With this, browser-laptop will notice when the tor daemon has come
back after muon executes ses.relaunchTor, without needing to call
setupTor again -- which might have had unpredictable consequences of
multiple simultaneous file system watchers and control connections to
tor.

fix #14584

Auditors: @diracdeltas

Test Plan:
1. Turn off your network.
2. Launch Brave.
3. Open a private tab with Tor enabled.
4. Check that the console reports `tor: daemon listens on ...`.
5. Wait 20sec for the connection error dialogue box to pop up.
6. Hit 'retry connection'.
7. Check that the console reports `tor: daemon listens on ...` _again_.
@riastradh-brave
Copy link
Contributor Author

fixed in #14585

@LaurenWags
Copy link
Member

LaurenWags commented Jul 2, 2018

Verified with macOS 10.12.6 using

  • 0.23.21 7240522
  • Muon 7.1.4
  • libchromiumcontent 67.0.3396.103

Verified on Windows x64 using

  • 0.23.24 a471718
  • Muon 7.1.6
  • libchromiumcontent 67.0.3396.103

Verified on Ubuntu 17.10 x64

  • 0.23.34 a471718
  • Muon 7.1.6
  • libchromiumcontent 67.0.3396.103

@srirambv
Copy link
Collaborator

QA blocked on windows because of #14630

@btlechowski
Copy link
Contributor

Blocked by #14630 on Linux

@riastradh-brave
Copy link
Contributor Author

@srirambv @btlechowski #14630 might be fixed now! Does that help?

@srirambv
Copy link
Collaborator

@riastradh-brave #14630 is not fixed yet. Still getting error when retry connection is clicked

@LaurenWags
Copy link
Member

LaurenWags commented Jul 11, 2018

This is now broken on macOS as well - was working in a previous build. User error, this is working with 0.23.33

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.