Issue metadata
Sign in to add a comment
|
Shared Workers are not correctly terminated when the browser shutdown. |
||||||||||||||||||||||
Issue descriptionMoved from issue 636377. https://codereview.chromium.org/2222063002/ added two CHECK()s which are called when the browser shutdown. [1] SharedWorkerServiceImpl::CheckAllWorkersTerminated() [2] RenderProcessHostImpl::CheckAllWorkersTerminated() These CHECKs() can hit by these steps: 1. Open two tabs (tab 1 and tab 2). 2. Open http://goo.gl/gl7xws in tab 1. 3. Open http://goo.gl/gl7xws in tab 2. 4. Click "startSharedWorker1" in tab 1. 5. Click "startSharedWorker1" in tab 2. 6. Click "startSharedWorker2" in tab 2. 7. Click "startSharedWorker2" in tab 1. 8. Close the window. [1] SharedWorkerServiceImpl::CheckAllWorkersTerminated() - Add "usleep(1000000);" in SharedWorkerRepository::documentDetached() before "Send(new ViewHostMsg_DocumentDetached(document));". This uleep simulates the slow main thread of the renderer process. [2] RenderProcessHostImpl::CheckAllWorkersTerminated() - Add "usleep(1000000);" in SharedWorkerRepository::documentDetached() before "Send(new ViewHostMsg_DocumentDetached(document));". This simulates the slow main thread of the renderer process. - Add "usleep(2000000);" in BrowserMainLoop::ShutdownThreadsAndCleanUp() before "ResetThread_IO(std::move(io_thread_));". This uleep simulates the slow IO thread of the browser process. To fix this issue, we have to refine the shutdown logic for shared worker.
,
Aug 22 2016
,
Aug 24 2016
,
Aug 24 2016
,
Aug 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/798a8b94726d3c439e6302998f31bc487f521ec8 commit 798a8b94726d3c439e6302998f31bc487f521ec8 Author: horo <horo@chromium.org> Date: Thu Aug 25 14:02:50 2016 Removes the references to shared workers from the all documents in being deleted frames. So that we can shut down any shared workers that are no longer referenced by active documents without waiting for DocumentDetached IPC message. This will fix the race condition reported in https://crbug.com/636377#c11 BUG= 639193 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2249173003 Cr-Commit-Position: refs/heads/master@{#414427} [modify] https://crrev.com/798a8b94726d3c439e6302998f31bc487f521ec8/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/798a8b94726d3c439e6302998f31bc487f521ec8/content/browser/shared_worker/shared_worker_host.cc [modify] https://crrev.com/798a8b94726d3c439e6302998f31bc487f521ec8/content/browser/shared_worker/shared_worker_host.h [modify] https://crrev.com/798a8b94726d3c439e6302998f31bc487f521ec8/content/browser/shared_worker/shared_worker_service_impl.cc [modify] https://crrev.com/798a8b94726d3c439e6302998f31bc487f521ec8/content/browser/shared_worker/shared_worker_service_impl.h [modify] https://crrev.com/798a8b94726d3c439e6302998f31bc487f521ec8/content/browser/shared_worker/worker_document_set.cc [modify] https://crrev.com/798a8b94726d3c439e6302998f31bc487f521ec8/content/browser/shared_worker/worker_document_set.h
,
Aug 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1109b15f8669b2c342d87e9ae6e5d8ebd669ada commit d1109b15f8669b2c342d87e9ae6e5d8ebd669ada Author: horo <horo@chromium.org> Date: Sun Aug 28 02:43:01 2016 Reland of Check that all shared workers are terminated. (patchset #2 id:170001 of https://codereview.chromium.org/2254963002/ ) Reason for revert: Shared Worker bug fix patch landed. https://codereview.chromium.org/2249173003/ BUG= 639193 Original issue's description: > Revert of Check that all shared workers are terminated. (patchset #1 id:1 of https://codereview.chromium.org/2222063002/ ) > > Reason for revert: > Reverting temporary instrumentation following investigation > > We want to get this in prior to dev branch. > > BUG=636377 > > Original issue's description: > > Check that all shared workers are terminated. > > > > Adds temporary checks to determine whether shared workers > > prevent RenderProcessHost(s) from cleaning up due to > > RenderProcessHostImpl::worker_ref_count_ > 0. > > > > BUG= 608049 > > > > Committed: https://crrev.com/529173d1bbeb53b3f9438648fad264b4db042721 > > Cr-Commit-Position: refs/heads/master@{#410730} > > TBR=atwilson@chromium.org,jam@chromium.org,horo@chromium.org,alokp@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG= 608049 > > Committed: https://crrev.com/463ac7e96bf55b106225481811b0db752284bbed > Cr-Commit-Position: refs/heads/master@{#412660} TBR=atwilson@chromium.org,jam@chromium.org,alokp@chromium.org,erikchen@chromium.org,manzagop@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=636377 Review-Url: https://codereview.chromium.org/2286003002 Cr-Commit-Position: refs/heads/master@{#414952} [modify] https://crrev.com/d1109b15f8669b2c342d87e9ae6e5d8ebd669ada/content/browser/browser_main_loop.cc [modify] https://crrev.com/d1109b15f8669b2c342d87e9ae6e5d8ebd669ada/content/browser/browser_process_sub_thread.cc [modify] https://crrev.com/d1109b15f8669b2c342d87e9ae6e5d8ebd669ada/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/d1109b15f8669b2c342d87e9ae6e5d8ebd669ada/content/browser/renderer_host/render_process_host_impl.h [modify] https://crrev.com/d1109b15f8669b2c342d87e9ae6e5d8ebd669ada/content/browser/shared_worker/shared_worker_service_impl.cc [modify] https://crrev.com/d1109b15f8669b2c342d87e9ae6e5d8ebd669ada/content/browser/shared_worker/shared_worker_service_impl.h
,
Aug 29 2016
Memo: $ git find-releases 798a8b94726d3c439e6302998f31bc487f521ec8 commit 798a8b94726d3c439e6302998f31bc487f521ec8 was: initially in 54.0.2840.0
,
Aug 29 2016
$ git find-releases d1109b15f8669b2c342d87e9ae6e5d8ebd669ada commit d1109b15f8669b2c342d87e9ae6e5d8ebd669ada was: initially in 55.0.2843.0
,
Aug 29 2016
'content::RenderProcessHostImpl::CheckAllWorkersTerminated' is the #1 browser crash on the latest canary(55.0.2843.0 - 26 crashes from 20 diff. clients) based on the available crash data. Marking this as RB-Beta for M-55, for now. Stack trace of '95f5082e00000000' of the mentioned magic siganture: Thread 0 CRASHED [EXCEPTION_ACCESS_VIOLATION_WRITE @ 0x00000000 ] MAGIC SIGNATURE THREAD 0x00007ffc12525afd (chrome.dll -render_process_host_impl.cc:792 ) content::RenderProcessHostImpl::CheckAllWorkersTerminated() 0x00007ffc1234429d (chrome.dll -browser_main_loop.cc:1112 ) content::BrowserMainLoop::ShutdownThreadsAndCleanUp() 0x00007ffc12347fac (chrome.dll -browser_main_runner.cc:211 ) content::BrowserMainRunnerImpl::Shutdown() 0x00007ffc123409f9 (chrome.dll -browser_main.cc:48 ) content::BrowserMain(content::MainFunctionParams const &) 0x00007ffc12809a3a (chrome.dll -content_main_runner.cc:786 ) content::ContentMainRunnerImpl::Run() 0x00007ffc11a238a6 (chrome.dll -chrome_main.cc:85 ) ChromeMain 0x00007ff648ea7366 (chrome.exe -main_dll_loader_win.cc:182 ) MainDllLoader::Launch(HINSTANCE__ *) 0x00007ff648ea252c (chrome.exe -chrome_exe_main_win.cc:250 ) wWinMain 0x00007ff649165bb9 (chrome.exe -exe_common.inl:255 ) __scrt_common_main_seh 0x00007ffc434a8363 (KERNEL32.DLL + 0x00008363 ) BaseThreadInitThunk 0x00007ffc447a5e90 (ntdll.dll + 0x00065e90 ) RtlUserThreadStart
,
Aug 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e60baa3aa1365c8c0002631ffe95c790e6a0ab1e commit e60baa3aa1365c8c0002631ffe95c790e6a0ab1e Author: horo <horo@chromium.org> Date: Tue Aug 30 06:01:15 2016 CHECK instance_info_.empty() in ServiceWorkerProcessManager destructor To verify that ServiceWorkerProcessManager doesn't prevent render process hosts from shutting down. BUG= 639193 ,636377 Review-Url: https://codereview.chromium.org/2289003002 Cr-Commit-Position: refs/heads/master@{#415193} [modify] https://crrev.com/e60baa3aa1365c8c0002631ffe95c790e6a0ab1e/content/browser/service_worker/service_worker_process_manager.cc
,
Sep 5 2016
,
Sep 5 2016
I'll merge this back to Issue 636377 since that was reopened.
,
Sep 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff commit 26c0d0b4d4a8a5c48d9311c4e65485bae762bfff Author: falken <falken@chromium.org> Date: Mon Sep 05 14:40:22 2016 For debugging, split worker ref count into service worker and shared worker counts This should help narrow down the cause of the nonzero ref count on shutdown. Also, move the ref counts members near the top of RenderProcessHostImpl, since the minidumps we are seeing don't have them in memory, while earlier parts of the object are there. BUG=636377, 639193 Review-Url: https://codereview.chromium.org/2312633002 Cr-Commit-Position: refs/heads/master@{#416545} [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/browser/renderer_host/render_process_host_impl.h [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/browser/service_worker/service_worker_process_manager.cc [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/browser/shared_worker/shared_worker_service_impl.cc [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/browser/site_per_process_browsertest.cc [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/public/browser/render_process_host.h [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/public/test/mock_render_process_host.cc [modify] https://crrev.com/26c0d0b4d4a8a5c48d9311c4e65485bae762bfff/content/public/test/mock_render_process_host.h |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by horo@chromium.org
, Aug 19 2016