Invalid process handles in ChildProcessData |
||||||||
Issue description
I'm hitting invalid handle failures when doing App Verifier testing with M69 Chrome (app verifier works with Chrome from M68 on). The call stack is:
04 KERNELBASE!GetProcessId
05 chrome!resource_coordinator::BrowserChildProcessWatcher::BrowserChildProcessLaunchedAndConnected
06 chrome!content::`anonymous namespace'::NotifyProcessLaunchedAndConnected
07 chrome!base::debug::TaskAnnotator::RunTask
08 chrome!base::MessageLoop::RunTask
09 chrome!base::MessageLoop::DoWork
0a chrome!base::MessagePumpForUI::DoRunLoop
0b chrome!base::MessagePumpWin::Run
0c chrome!base::RunLoop::Run
0d chrome!ChromeBrowserMainParts::MainMessageLoopRun
0e chrome!content::BrowserMainLoop::RunMainMessageLoopParts
0f chrome!content::BrowserMainRunnerImpl::Run
10 chrome!content::BrowserMain
11 chrome!content::RunBrowserProcessMain
12 chrome!content::ContentMainRunnerImpl::Run
13 chrome!service_manager::Main
14 chrome!content::ContentMain
15 chrome!ChromeMain
16 chrome_exe!MainDllLoader::Launch
17 chrome_exe!wWinMain
Roughly speaking the failure happens because ChildProcessData::handle is just a HANDLE on Windows and since ChildProcessData doesn't own it there is no guarantee that it is valid by the time we get around to using it. I'm hitting this failure repeatedly when I launch Chrome with App Verifier enabled.
Not surprisingly the handle error can happen with other listeners, such as this one:
04 KERNELBASE!GetProcessId
05 chrome!content::AudioServiceListener::BrowserChildProcessHostDisconnected
06 chrome!content::`anonymous namespace'::NotifyProcessHostDisconnected
07 chrome!base::debug::TaskAnnotator::RunTask
I started trying to fix this, primarily by adding this block to ChildProcessData and using GetHandle/SetHandle where needed:
#if defined(OS_WIN)
// Must be a scoped handle on Windows or else there are no guarantees the
// handle will still be valid when used.
base::win::ScopedHandle handle;
base::ProcessHandle GetHandle() const { return handle.Get(); }
void SetHandle(base::ProcessHandle process ) {
handle = base::win::ScopedHandle(process);
}
#else
base::ProcessHandle handle;
base::ProcessHandle GetHandle() const { return handle; }
void SetHandle(base::ProcessHandle handle) { handle = process; }
#endif
however it feels like ScopedHandle is fighting me all the way. In particular, since ScopedHandle can't be copied the BindOnce call fails here:
BrowserChildProcessHostImpl::~BrowserChildProcessHostImpl() {
g_child_process_list.Get().remove(this);
if (notify_child_disconnected_) {
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
base::BindOnce(&NotifyProcessHostDisconnected, data_));
}
}
I could make ChildProcessData explicitly copyable through handle duplication but it feels like I might be heading in the wrong direction with that.
The process in question is always the Data Decoder Service, process_type 6 (PROCESS_TYPE_UTILITY). It sounds like either this is a very short-lived process or else it is launched in an unusual way which triggers this problem.
For a minimal repro of the issue build Chrome with https://codereview.chromium.org/2774463002 and launch it under a multi-process debugger - windbg -g -G -o does the trick.
,
Jul 20
Well it's not launched in an unusual way, but it is probably very short-lived most of the time. Making ChildProcessData copy correct sounds reasonable. I wonder if we could justify migrating mojo::core::ScopedProcessHandle[1] to base and just make ChildProcessData move-only? [1] https://cs.chromium.org/chromium/src/mojo/core/scoped_process_handle.h?rcl=73c48e168bfaab6601fd0a6cfd54834d8673112e&l=30
,
Jul 20
I'll poke at my fix a bit more to see what it takes to get it working. I don't think ScopedHandle versus ScopedProcessHandle makes much of a difference so I'll stick with ScopedHandle for now. There is presumably some risk of handle reuse causing odd behavior but I'm not sure what the frequency or likely consequences of that would be and therefore I can't guess at the severity. It's clear from the try jobs on the CL where I add a check to GetProcId (crrev.com/c/1144477 - I linked to the wrong CL in the initial report) that the bug reproes very easily, but in most cases it just means we fail to get a PID when we want it.
,
Jul 20
This investigation is revealing a number of places where we are doing dodgy things with process handles - more raw handles than I am comfortable with. And, I've found one code construct that may be made worse by doing proper handle management - see ChildProcessTaskProvider::DeleteTask(base::ProcessHandle handle) and crbug.com/611067. The problem is that the DeleteTask function assumes that processes can be identified by their handles, but using ScopedHandle objects or equivalent means having multiple handles to the same process, so the handles will compare as being different despite referring to the same process.
,
Jul 20
Sounds like it should just be using base::GetProcId in that case.
,
Jul 20
Also I think the main reason to consider something like ScopedProcessHandle is that ScopedHandle is Windows-only, but a great deal of code unsafely passing/stashing base::ProcessHandle is cross-platform.
,
Jul 23
> the main reason to consider something like ScopedProcessHandle is that ScopedHandle is Windows-only Agreed. I realized that after noticing that ProcessMetricsMetadata also has a raw base::ProcessHandle which potentially means a lot of replicated fixes. I created an initial fix for this bug that resolves the issues. The CL is crrev.com/c/1145767. It needs more cleanup/thought, including: - Could use ScopedProcessHandle, or could postpone that for a later change - Many of the references to ScopedProcessHandle::handle are to check whether it is NULL so a helper function for that should be added - Need to consider whether the DeleteTask bug needs fixing at the same time - there are only eight references to tasks_by_handle_ so that should be straightforward and worthwhile The current fix avoids the failures that were detected even without App Verifier using the CHECK added in crrev.com/c/1144477. I just updated the CL to handle NULL process handles - try jobs with that update are running now.
,
Jul 23
,
Jul 24
I've iterated on my initial fix so that it compiles and passes tests on all platforms, resolving the original issue. Regarding the possible cleanups I mentioned in comment #7: - I have postponed using ScopedProcessHandle - I added a ChildProcessData::IsHandleValid function and use it where appropriate - I cleaned up tasks_by_handle_ to be tasks_by_processid_ I also made ChildProcessData::handle private in order to force use of the GetHandle()/SetHandle() methods.
,
Jul 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c31de3260f48a6c197d698f8f2e295cd9f04d49b commit c31de3260f48a6c197d698f8f2e295cd9f04d49b Author: Bruce Dawson <brucedawson@chromium.org> Date: Thu Jul 26 20:24:07 2018 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} [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/base/process/process_handle_win.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/android/compositor/compositor_view.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/android/feedback/process_id_feedback_source.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/chrome_content_browser_client_browsertest_chromeos.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/chrome_plugin_browsertest.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/chromeos/app_mode/app_session.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/extensions/api/processes/processes_api.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/image_decoder_browsertest.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/memory_details.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/performance_monitor/performance_monitor.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/resource_coordinator/browser_child_process_watcher.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/task_manager/providers/child_process_task.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/task_manager/providers/child_process_task_provider.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/task_manager/providers/child_process_task_provider.h [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/task_manager/providers/child_process_task_unittest.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/task_manager/providers/render_process_host_task_provider.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/ui/hung_plugin_tab_helper.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/chrome/browser/ui/webui/memory_internals_ui.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/components/browser_watcher/exit_code_watcher_win_unittest.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/components/crash/content/browser/child_exit_observer_android.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/components/heap_profiling/client_connection_manager.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/components/nacl/browser/nacl_broker_host_win.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/components/nacl/browser/nacl_process_host.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/browser/browser_child_process_host_impl.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/browser/histogram_controller.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/browser/ppapi_plugin_process_host.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/browser/renderer_host/media/audio_service_listener.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/browser/renderer_host/media/audio_service_listener_unittest.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/browser/service_manager/service_manager_context.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/public/browser/BUILD.gn [add] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/public/browser/child_process_data.cc [modify] https://crrev.com/c31de3260f48a6c197d698f8f2e295cd9f04d49b/content/public/browser/child_process_data.h
,
Jul 26
Should be fixed.
,
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
,
Jul 30
Reopening since the fix was reverted.
,
Jul 30
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02f07de804a9c247c5b6405f0d7dde88da6bee3c commit 02f07de804a9c247c5b6405f0d7dde88da6bee3c Author: Bruce Dawson <brucedawson@chromium.org> Date: Tue Jul 31 17:28:26 2018 Reland "Fix invalid handle errors found with App Verifier" This is a reland of c31de3260f48a6c197d698f8f2e295cd9f04d49b. The CHECK that was triggering (crbug.com/868341) has been removed but the change is otherwise the same. The CHECK will be readded once all of the bugs that it reveals are fixed (crbug.com/869154) 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} Bug: 417532, 821453, 865805 , 868341, 869154 Change-Id: I022982f447cce4649618f4413e960f5639ad750a Reviewed-on: https://chromium-review.googlesource.com/1155498 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Bruce Dawson <brucedawson@chromium.org> Cr-Commit-Position: refs/heads/master@{#579454} [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/android/compositor/compositor_view.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/android/feedback/process_id_feedback_source.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/chrome_content_browser_client_browsertest_chromeos.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/chrome_plugin_browsertest.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/chromeos/app_mode/app_session.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/extensions/api/processes/processes_api.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/image_decoder_browsertest.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/memory_details.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/performance_monitor/performance_monitor.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/resource_coordinator/browser_child_process_watcher.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/task_manager/providers/child_process_task.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/task_manager/providers/child_process_task_provider.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/task_manager/providers/child_process_task_provider.h [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/task_manager/providers/child_process_task_unittest.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/task_manager/providers/render_process_host_task_provider.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/ui/hung_plugin_tab_helper.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/chrome/browser/ui/webui/memory_internals_ui.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/components/browser_watcher/exit_code_watcher_win_unittest.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/components/crash/content/browser/child_exit_observer_android.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/components/heap_profiling/client_connection_manager.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/components/nacl/browser/nacl_broker_host_win.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/components/nacl/browser/nacl_process_host.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/browser/browser_child_process_host_impl.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/browser/histogram_controller.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/browser/ppapi_plugin_process_host.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/browser/renderer_host/media/audio_service_listener.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/browser/renderer_host/media/audio_service_listener_unittest.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/browser/service_manager/service_manager_context.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/public/browser/BUILD.gn [add] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/public/browser/child_process_data.cc [modify] https://crrev.com/02f07de804a9c247c5b6405f0d7dde88da6bee3c/content/public/browser/child_process_data.h
,
Jul 31
Fixed again, and this time the fix should stick because the CHECK was not added to GetProcId (yet).
,
Oct 16
Issue 895436 has been merged into this issue.
,
Oct 16
There are a couple of crashes with this signature coming from audio service listener, most of them in M68 (~100), and some from M69 (~10): https://crash.corp.google.com/browse?q=EXISTS+%28SELECT+1+FROM+UNNEST%28CrashedStackTrace.StackFrame%29+WHERE+FunctionName%3D%27content%3A%3AAudioServiceListener%3A%3ABrowserChildProcessHostDisconnected%28content%3A%3AChildProcessData+const+%26%29%27%29#-propertyselector,samplereports,productname:1000,+productversion,channel,-magicsignature:50,-magicsignature2:50,-stablesignature:50,osversion The fix in #15 will be included in M70. Not sure if the fix needs merging to M69?
,
Oct 16
,
Oct 17
There are a couple of crashes with this signature from M70, but they are from the same client, they have hmpalert.dll and NamespaceToEvents.dll loaded, and they can probably be ignored. The ones from M69 are interesting because EXCEPTION_INVALID_HANDLE is normally suppressed so I would have expected the bad GetProcId call (which tail calls straight to KERNELBASE!GetProcessId) to fail silently, rather than crashing. That said, since M70 is rolling out no and since the crash rate is very low there is no need to merge to M69. Thank you for the report. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by brucedaw...@chromium.org
, Jul 20