New issue
Advanced search Search tips

Issue 868384 link

Starred by 1 user
Status: Fixed
Owner:
sky@chromium.org
Closed: Jul 27
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: SingleClientAppsSyncTest.StartWithSomePlatformApps



Sign in to add a comment

SingleClientAppsSyncTest.StartWithSomePlatformApps is Flaky

Project Member Reported by Findit, Jul 27

Comment 2 by sky@chromium.org, Jul 27

Owner: sky@chromium.org
Status: Started (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 27 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c03a192a88c2514a21121251fe6ea0300d0d2353

commit c03a192a88c2514a21121251fe6ea0300d0d2353
Author: Scott Violet <sky@chromium.org>
Date: Fri Jul 27 15:38:34 2018

Revert "Fix invalid handle errors found with App Verifier"

This reverts commit c31de3260f48a6c197d698f8f2e295cd9f04d49b.

Reason for revert: Findit thinks this is the cause for SingleClientAppsSyncTest.StartWithSomePlatformApps is Flaky becoming flaky. Debug output from the bot shows CHECK in process_handle_win:

[5744:6016:0727/011726.878:FATAL:process_handle_win.cc(29)] Check failed: result != 0 || GetLastError() != ERROR_INVALID_HANDLE. process handle = 00000FB4
Backtrace:
	base::debug::StackTrace::StackTrace [0x73D7A1D6+102]
	base::debug::StackTrace::StackTrace [0x73D7927B+27]
	logging::LogMessage::~LogMessage [0x73DDEFA4+148]
	base::GetProcId [0x73EC8E8E+254]
	content::ChildConnection::IOThreadContext::SetProcessHandleOnIOThread [0x617A7C58+232]
	base::internal::FunctorTraits<void (__thiscall content::ChildConnection::IOThreadContext::*)(void *),void>::Invoke<void (__thiscall content::ChildConnection::IOThreadContext::*)(void *),scoped_refptr<content::ChildConnection::IOThreadContext>,void *> [0x617A8001+81]
	base::internal::InvokeHelper<0,void>::MakeItSo<void (__thiscall content::ChildConnection::IOThreadContext::*)(void *),scoped_refptr<content::ChildConnection::IOThreadContext>,void *> [0x617A7F0C+124]
	base::internal::Invoker<base::internal::BindState<void (__thiscall content::ChildConnection::IOThreadContext::*)(void *),scoped_refptr<content::ChildConnection::IOThreadContext>,void *>,void __cdecl(void)>::RunImpl<void (__thiscall content::ChildConnectio [0x617A7E3F+111]
	base::internal::Invoker<base::internal::BindState<void (__thiscall content::ChildConnection::IOThreadContext::*)(void *),scoped_refptr<content::ChildConnection::IOThreadContext>,void *>,void __cdecl(void)>::RunOnce [0x617A7CE4+84]
	base::OnceCallback<void __cdecl(void)>::Run [0x73D1CA90+80]
	base::debug::TaskAnnotator::RunTask [0x73D7E9F3+1075]
	base::MessageLoop::RunTask [0x73E19A64+884]
	base::MessageLoop::DeferOrRunPendingTask [0x73E1A249+73]
	base::MessageLoop::DoWork [0x73E1A5BA+442]
	base::MessagePumpForIO::DoRunLoop [0x73E2775C+28]
	base::MessagePumpWin::Run [0x73E25CA7+119]
	base::MessageLoop::Run [0x73E19366+486]
	base::RunLoop::Run [0x73EDF518+488]
	base::Thread::Run [0x73FF8EF7+375]
	content::BrowserProcessSubThread::IOThreadRun [0x61B456A0+48]
	content::BrowserProcessSubThread::Run [0x61B45589+313]
	base::Thread::ThreadMain [0x73FF97BA+1402]
	base::PlatformThread::GetCurrentThreadPriority [0x73FEBC76+934]
	BaseThreadInitThunk [0x74AD336A+18]
	RtlInitializeExceptionChain [0x771F9902+99]
	RtlInitializeExceptionChain [0x771F98D5+54]

Original change's description:
> Fix invalid handle errors found with App Verifier
> 
> ChildProcessData was holding a raw ProcessHandle which meant that there
> was no guarantee that the handle would be valid when it was used. This
> change switches to a ScopedHandle on Windows, with the associated
> duplication of the handle, so that it is guaranteed to be valid. This
> fixes a frequently encountered use of an invalid handle that was
> detected by Application Verifier. This change also adds a CHECK to
> base::GetProcId to check for use of invalid handles even when not using
> Application Verifier.
> 
> ChildProcessData::handle is now a private member so all references to
> it were updated to use accessors.
> 
> When ChildProcessData::SetHandle is called the handle is duplicated.
> When a ChildProcessData object needs to be copied an explicit
> Duplicate function is called.
> 
> An IsHandleValid helper function was added.
> 
> An invalid test was removed because it was triggering the CHECK in
> base::GetProcId.
> 
> Bug: 417532,821453, 865805 
> Change-Id: I7e38140335c0140536919341f011f144f150c88f
> Reviewed-on: https://chromium-review.googlesource.com/1145767
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578419}

TBR=brucedawson@chromium.org,jochen@chromium.org

Change-Id: I8569cb32ec0cd9cbd99ecf6d5bba7feb672a88cd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 417532, 821453,  865805 ,  868384 
Reviewed-on: https://chromium-review.googlesource.com/1152374
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578652}
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/base/process/process_handle_win.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/android/compositor/compositor_view.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/android/feedback/process_id_feedback_source.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/chrome_content_browser_client_browsertest_chromeos.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/chrome_plugin_browsertest.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/chromeos/app_mode/app_session.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/extensions/api/processes/processes_api.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/image_decoder_browsertest.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/memory_details.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/performance_monitor/performance_monitor.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/resource_coordinator/browser_child_process_watcher.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/task_manager/providers/child_process_task.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/task_manager/providers/child_process_task_provider.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/task_manager/providers/child_process_task_provider.h
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/task_manager/providers/child_process_task_unittest.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/task_manager/providers/render_process_host_task_provider.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/ui/hung_plugin_tab_helper.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/chrome/browser/ui/webui/memory_internals_ui.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/components/browser_watcher/exit_code_watcher_win_unittest.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/components/crash/content/browser/child_exit_observer_android.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/components/heap_profiling/client_connection_manager.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/components/nacl/browser/nacl_broker_host_win.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/components/nacl/browser/nacl_process_host.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/browser/histogram_controller.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/browser/ppapi_plugin_process_host.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/browser/renderer_host/media/audio_service_listener.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/browser/renderer_host/media/audio_service_listener_unittest.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/public/browser/BUILD.gn
[delete] https://crrev.com/e30800ae4a616c0e3fc040c57a7fd20aa7b3f7c1/content/public/browser/child_process_data.cc
[modify] https://crrev.com/c03a192a88c2514a21121251fe6ea0300d0d2353/content/public/browser/child_process_data.h

Comment 4 by sky@chromium.org, Jul 27

Status: Fixed (was: Started)
►

Sign in to add a comment