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

Prevent SuicideOnChannelErrorFilter to be added to tor_launcher utility process #631

Merged
merged 1 commit into from
Jul 17, 2018

Conversation

darkdh
Copy link
Member

@darkdh darkdh commented Jul 12, 2018

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

…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
@darkdh darkdh self-assigned this Jul 12, 2018
@darkdh
Copy link
Member Author

darkdh commented Jul 13, 2018

@bridiver suggested using kqueue like WaitForChildToDie in base/process/kill_mac.cc
Basically putting a parent death handler in base::LaunchOptions::PreExecDelegate
Current PR still doesn't address the case that you kill utility process directly and tor process will still be left alone.

I'm working on it

@bridiver
Copy link
Collaborator

just making a note that we should revisit this in brave-core and look at other options because this can still leave a zombie process in other cases

@darkdh darkdh merged commit fb9327a into tor_browser_context Jul 17, 2018
darkdh added a commit that referenced this pull request Jul 17, 2018
Prevent `SuicideOnChannelErrorFilter` to be added to tor_launcher utility process
@darkdh darkdh deleted the tor_cleanup branch July 18, 2018 18:07
darkdh added a commit that referenced this pull request Jul 18, 2018
Prevent `SuicideOnChannelErrorFilter` to be added to tor_launcher utility process
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants