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

Tor process remains alive after crash #14636

Closed
mrphs opened this issue Jul 3, 2018 · 5 comments
Closed

Tor process remains alive after crash #14636

mrphs opened this issue Jul 3, 2018 · 5 comments
Assignees
@darkdh
Labels
feature/tor OS/macOS priority/P2 Crashes. Loss of data. Severe memory leak. QA/checked-macOS QA/test-plan-specified release/not-blocking release-notes/include
Milestone
0.23.x Release 4 ...

Comments

@mrphs
Copy link

mrphs commented Jul 3, 2018 •
edited by srirambv
Loading

Test plan

See #14636 (comment)

Description

I just had my Brave installation crash and realized it wouldn't make any connection to the Tor network on startup anymore. Took a quick look at the processes alive, and surely enough, tor was still sitting there. Unfortunately, I haven't had a chance to look at the code but I'm blind guessing when the browser is re-launched, it doesn't look to see if the tor process was properly killed or if it's still running and attach to it, instead of trying to re-run the same instance of tor, most likely resulting in the "Tor is already running" error but the browser wouldn't know what to do with it. Again, just blind guessing before actually reading the code :)

Steps to Reproduce

  1. Run Brave
  2. Do something to make it crash
  3. Re-launch the browser

Actual result:

Brave returns error as if we weren't able to connect to the Tor network, while firewall shows Tor is communicating with the network and is alive. Clicking on "retry Tor connection" doesn't fix it either.

Expected result:

  • Brave should look to see if its Tor process is running at the startup and either kill or re-attach to it.
  • "Retry Tor connection" button should actually try to kill Brave's Tor process and retry connecting. It seems like it's not doing what it's expected of, in this scenario.
  • Maybe Brave should mark its Tor instance as tor.brave or something similar so it's easily distinguishable from other Tor instances from other software (ie Tor Browser or OnionShare)

Reproduces how often:
Only tried it once.

Brave Version

Name Version
Brave 0.23.19
V8 6.7.288.46
rev 178c3fb
Muon 7.1.3
OS Release 17.6.0
Update Channel Release
OS Architecture x64
OS Platform macOS
Node.js 7.9.0
Tor 0.3.3.7 (git-035a35178c92da94)
Brave Sync v1.4.2
libchromiumcontent 67.0.3396.87
@riastradh-brave
Copy link
Contributor

We made some attempt to kill the process, but apparently the Chromium kill_on_parent_death launch option works only on Linux:

https://github.com/brave/muon/blob/73170e0d9072bf60ee45f6e06ecb4449ee22d2fa/brave/utility/tor/tor_launcher_impl.cc#L159-L161

@darkdh We need some way to make sure the process gets killed on macOS too.

@darkdh
Copy link
Member

darkdh commented Jul 3, 2018

Also we handle it in normal exit by base::EnsureProcessTerminated
https://github.com/brave/muon/blob/73170e0d9072bf60ee45f6e06ecb4449ee22d2fa/brave/utility/tor/tor_launcher_impl.cc#L106-L114
so we will need to take care of crash

@riastradh-brave
Copy link
Contributor

We will see if a signal handler in the utility process is enough to make sure we kill the tor process.

@alexwykoff alexwykoff added the priority/P2 Crashes. Loss of data. Severe memory leak. label Jul 10, 2018
@darkdh darkdh mentioned this issue Jul 12, 2018
Merged
@darkdh
Copy link
Member

darkdh commented Jul 12, 2018

Note for QA, this can only reproduce on Mac but you can always check on platforms
Steps to verify:

  1. Open Brave with BRAVE_ENABLE_DEBUG_MENU=true
  2. Open a tor tab
  3. Menu->Debug->Crash main process
  4. Open Brave again
  5. Open a tor tab
  6. You should be able to connect to tor network without error

@srirambv srirambv mentioned this issue Jul 21, 2018
Closed
darkdh added a commit to brave/muon that referenced this issue Jul 25, 2018
@darkdh
Prevent SuicideOnChannelErrorFilter to be added to tor_launcher uti…
e556739 
…lity process

fix brave/browser-laptop#14636

SuicideOnChannelErrorFilter calls exit in OnChannelError() this will
cause other endpoints listener can't finish their cleanup when pipe error
happens(browser process crashed or be killed) and
SuicideOnChannelErrorFilter::OnChannelError happens to be called before others.
This should be fine for most of the cases but not tor_launcher service.
`TorLauncher` requires StrongBinding::OnConnectionError to delete itself
so that `~TorLauncher` will get called and terminate tor process.

This should only happens on MacOS, SuicideOnChannelErrorFilter is
guarded by OS_POSIX and Linux has `prctl(PR_SET_PDEATHSIG, SIGKILL)`
so tor process will receive SIGKILL when tor_launcher utility process terminates
prematurely

Auditors: @riastradh-brave, @bridiver, @bbondy
@LaurenWags LaurenWags mentioned this issue Jul 25, 2018
Closed
@LaurenWags
Copy link
Member

Verified with macOS 10.12.6 using

  • 0.23.72 c3b1cac
  • Muon 8.0.2
  • libchromiumcontent 68.0.3440.75

Also verified on 10.13 by @kjozwiak

@kjozwiak kjozwiak mentioned this issue Jul 31, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@darkdh darkdh

Labels
feature/tor OS/macOS priority/P2 Crashes. Loss of data. Severe memory leak. QA/checked-macOS QA/test-plan-specified release/not-blocking release-notes/include
Projects
None yet
Milestone
0.23.x Release 4 w/ Chromium 68 (Rele...
Development

No branches or pull requests
7 participants
@mrphs @alexwykoff @kjozwiak @darkdh @srirambv @LaurenWags @riastradh-brave