diff options
| author | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-07 20:20:51 +0000 |
|---|---|---|
| committer | jam@chromium.org <jam@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-09-07 20:20:51 +0000 |
| commit | e2024785888274aace77633521b0f080c0a1ae4d (patch) | |
| tree | 97dde341e0909f95f230753e189877235989d95a /content/browser/child_process_launcher.cc | |
| parent | 73767593af69baf76c02ab67251b4258948df3b7 (diff) | |
| download | chromium_src-e2024785888274aace77633521b0f080c0a1ae4d.zip chromium_src-e2024785888274aace77633521b0f080c0a1ae4d.tar.gz chromium_src-e2024785888274aace77633521b0f080c0a1ae4d.tar.bz2 | |
Fix race condition in ChildProcessLauncher. r79308 made process_ used on CHILD_PROCESS_LAUNCHER thread, even though it's not thread safe. I switched it to pass the handle directly, and made all the methods that run on CHILD_PROCESS_LAUNCHER static to make this clearer.
This also makes BrowserRenderProcessHost handle the case where ChildProcessLauncher fails, which it should (just like BrowserChildProcessHost does). It's another error why the zygote is failing sometimes.
Review URL: http://codereview.chromium.org/7739003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@100002 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content/browser/child_process_launcher.cc')
| -rw-r--r-- | content/browser/child_process_launcher.cc | 30 |
1 files changed, 17 insertions, 13 deletions
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc index 0892b66..c3d32a0 100644 --- a/content/browser/child_process_launcher.cc +++ b/content/browser/child_process_launcher.cc @@ -69,9 +69,10 @@ class ChildProcessLauncher::Context BrowserThread::PostTask( BrowserThread::PROCESS_LAUNCHER, FROM_HERE, - NewRunnableMethod( - this, + NewRunnableFunction( &Context::LaunchInternal, + make_scoped_refptr(this), + client_thread_id_, #if defined(OS_WIN) exposed_dir, #elif defined(OS_POSIX) @@ -101,7 +102,10 @@ class ChildProcessLauncher::Context Terminate(); } - void LaunchInternal( + static void LaunchInternal( + // |this_object| is NOT thread safe. Only use it to post a task back. + scoped_refptr<Context> this_object, + BrowserThread::ID client_thread_id, #if defined(OS_WIN) const FilePath& exposed_dir, #elif defined(OS_POSIX) @@ -194,10 +198,10 @@ class ChildProcessLauncher::Context #endif // else defined(OS_POSIX) BrowserThread::PostTask( - client_thread_id_, FROM_HERE, + client_thread_id, FROM_HERE, NewRunnableMethod( - this, - &ChildProcessLauncher::Context::Notify, + this_object.get(), + &Context::Notify, #if defined(OS_POSIX) && !defined(OS_MACOSX) use_zygote, #endif @@ -247,7 +251,7 @@ class ChildProcessLauncher::Context BrowserThread::PostTask( BrowserThread::PROCESS_LAUNCHER, FROM_HERE, NewRunnableFunction( - &ChildProcessLauncher::Context::TerminateInternal, + &Context::TerminateInternal, #if defined(OS_POSIX) && !defined(OS_MACOSX) zygote_, #endif @@ -255,9 +259,10 @@ class ChildProcessLauncher::Context process_.set_handle(base::kNullProcessHandle); } - void SetProcessBackgrounded(bool background) { - DCHECK(!starting_); - process_.SetProcessBackgrounded(background); + static void SetProcessBackgrounded(base::ProcessHandle handle, + bool background) { + base::Process process(handle); + process.SetProcessBackgrounded(background); } static void TerminateInternal( @@ -375,10 +380,9 @@ base::TerminationStatus ChildProcessLauncher::GetChildTerminationStatus( void ChildProcessLauncher::SetProcessBackgrounded(bool background) { BrowserThread::PostTask( BrowserThread::PROCESS_LAUNCHER, FROM_HERE, - NewRunnableMethod( - context_.get(), + NewRunnableFunction( &ChildProcessLauncher::Context::SetProcessBackgrounded, - background)); + GetHandle(), background)); } void ChildProcessLauncher::SetTerminateChildOnShutdown( |