New issue
Advanced search Search tips

Issue 878366 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug
Proj-Servicification



Sign in to add a comment

Crash 0x00007fffcff1c364 (chrome.dll -chrome_extensions_api_client.cc:129 ) extensions::ChromeExtensionsAPIClient::ShouldHideBrowserNetworkRequest(extensions::WebRequestInfo const &)

Project Member Reported by dxie@chromium.org, Aug 28

Issue description

See a bunch of crashes on this new signature:

https://crash.corp.google.com/browse?q=product_name%3D%27Chrome%27+AND+expanded_custom_data.ChromeCrashProto.channel%3D%27canary%27+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27InstantIOContext%3A%3AIsInstantProcess%27+AND+EXISTS+%28SELECT+1+FROM+UNNEST%28expanded_custom_data.ChromeCrashProto.experiments.ids%29+expanded_custom_data_ChromeCrashProto_experiments_ids+WHERE+expanded_custom_data_ChromeCrashProto_experiments_ids%3D%27e111fcd-3f4a17df%27%29&stbtiq=&reportid=&index=0

Report IDa57ad8f93a5fc3b4
Product, versionChrome, 70.0.3535.0
Process typebrowser
Magic SignatureInstantIOContext::IsInstantProcess
edit bugs&comments
Stable Signaturebase::SupportsUserData::GetUserData-cd7cfaf6edit bugs&comments
Report TimeTue, 28 Aug 2018 12:02:44 GMT
Upload TimeTue, 28 Aug 2018 12:02:53 GMT
Processed TimeTue, 28 Aug 2018 12:03:34 GMT
Process uptime32 min, 27 sec, 0 ms
Client ID7a8ec54d-8e7d-4c46-8cb6-29522ddf1de4
FilesminidumpDownloadReprocess this minidump
In shutdownfalse
Thread 3 Chrome_IOThread (id: 0x3588) CRASHED [EXCEPTION_ACCESS_VIOLATION_READ @ 0xffffffffffffffff ] MAGIC SIGNATURE THREAD
Stack Quality84%Show frame trust levels
0x00007fffcfcb68c8	(chrome.dll -supports_user_data.cc:19 )	base::SupportsUserData::GetUserData(void const *)
0x00007fffcff1c39f	(chrome.dll -instant_io_context.cc:97 )	InstantIOContext::IsInstantProcess(content::ResourceContext *,int)
0x00007fffcff1c364	(chrome.dll -chrome_extensions_api_client.cc:129 )	extensions::ChromeExtensionsAPIClient::ShouldHideBrowserNetworkRequest(extensions::WebRequestInfo const &)
0x00007fffcff1c0db	(chrome.dll -web_request_permissions.cc:251 )	IsSensitiveRequest(extensions::WebRequestInfo const &,bool,bool)
0x00007fffcff1bd5d	(chrome.dll -web_request_permissions.cc:300 )	WebRequestPermissions::HideRequest(extensions::InfoMap const *,extensions::WebRequestInfo const &)
0x00007fffcff5a629	(chrome.dll -web_request_api.cc:1252 )	extensions::ExtensionWebRequestEventRouter::OnErrorOccurred(void *,extensions::InfoMap const *,extensions::WebRequestInfo const *,bool,int)
0x00007fffd0cb02f3	(chrome.dll -web_request_proxying_url_loader_factory.cc:523 )	extensions::WebRequestProxyingURLLoaderFactory::InProgressRequest::OnRequestError(network::URLLoaderCompletionStatus const &)
0x00007fffcfc65015	(chrome.dll -interface_endpoint_client.cc:326 )	mojo::InterfaceEndpointClient::NotifyError(base::Optional<mojo::DisconnectReason> const &)
0x00007fffcfc64f5f	(chrome.dll -multiplex_router.cc:789 )	mojo::internal::MultiplexRouter::ProcessNotifyErrorTask(mojo::internal::MultiplexRouter::Task *,mojo::internal::MultiplexRouter::ClientCallBehavior,base::SequencedTaskRunner *)
0x00007fffcfbdeeb7	(chrome.dll -multiplex_router.cc:700 )	mojo::internal::MultiplexRouter::ProcessTasks(mojo::internal::MultiplexRouter::ClientCallBehavior,base::SequencedTaskRunner *)
0x00007fffcfc3f685	(chrome.dll -multiplex_router.cc:674 )	mojo::internal::MultiplexRouter::OnPipeConnectionError(bool)
0x00007fffcfc64ac1	(chrome.dll -connector.cc:550 )	mojo::Connector::HandleError(bool,bool)
0x00007fffcfc4d156	(chrome.dll -simple_watcher.cc:273 )	mojo::SimpleWatcher::OnHandleReady(int,unsigned int,mojo::HandleSignalsState const &)
0x00007fffcfc3e77e	(chrome.dll -simple_watcher.cc:105 )	mojo::SimpleWatcher::Context::Notify(unsigned int,MojoHandleSignalsState,unsigned int)
0x00007fffcfc3e5e7	(chrome.dll -simple_watcher.cc:55 )	mojo::SimpleWatcher::Context::CallNotify(MojoTrapEvent const *)
0x00007fffcfc3e548	(chrome.dll -watcher_dispatcher.cc:90 )	mojo::core::WatcherDispatcher::InvokeWatchCallback(unsigned __int64,unsigned int,mojo::core::HandleSignalsState const &,unsigned int)
0x00007fffcfc3e4c6	(chrome.dll -watch.cc:78 )	mojo::core::Watch::InvokeCallback(unsigned int,mojo::core::HandleSignalsState const &,unsigned int)
0x00007fffcfbda754	(chrome.dll -request_context.cc:72 )	mojo::core::RequestContext::~RequestContext()
0x00007fffd029cf76	(chrome.dll -node_channel.cc:714 )	mojo::core::NodeChannel::OnChannelError(mojo::core::Channel::Error)
0x00007fffd0659fe1	(chrome.dll -channel_win.cc:209 )	mojo::core::`anonymous namespace'::ChannelWin::OnIOCompleted
0x00007fffcfb6dc6f	(chrome.dll -message_pump_win.cc:552 )	base::MessagePumpForIO::WaitForIOCompletion(unsigned long,base::MessagePumpForIO::IOHandler *)
0x00007fffcfb6d88c	(chrome.dll -message_pump_win.cc:517 )	base::MessagePumpForIO::DoRunLoop()
0x00007fffcfb6d72d	(chrome.dll -message_pump_win.cc:52 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x00007fffcfb6d490	(chrome.dll -run_loop.cc:102 )	base::RunLoop::Run()
0x00007fffcfb6d425	(chrome.dll -browser_process_sub_thread.cc:175 )	content::BrowserProcessSubThread::IOThreadRun(base::RunLoop *)
0x00007fffcfb6ad9a	(chrome.dll -thread.cc:357 )	base::Thread::ThreadMain()
0x00007fffd0d900e6	(chrome.dll -platform_thread_win.cc:100 )	base::`anonymous namespace'::ThreadFunc
0x00007ff833263033	(KERNEL32.DLL + 0x00013033 )	BaseThreadInitThunk
0x00007ff833591430	(ntdll.dll + 0x00071430 )	RtlUserThreadStart

clark, can you take a look?
 
Owner: cduvall@chromium.org
Status: Assigned (was: Available)
My best guess is that this happens when:

1) Request is made
2) Profile is destroyed, which destroys ResourceContext
3) WebRequestAPI, which is a BrowserContextKeyedAPI, gets destroyed because of that
4) WebRequestAPI posts a task to destroy proxies: https://cs.chromium.org/chromium/src/extensions/browser/api/web_request/web_request_api.cc?l=519&rcl=da3d369acf18c339183b77840d42a4c96cb00d72
5) Request client is destroyed, causing the WebRequestProxyingURLLoaderFactory::InProgressRequest::OnRequestError handler to run which uses ResourceContext which has already be destroyed

Since the proxies have to wait for the task to be destroyed, some of the BrowserContext scoped data held by them can be invalid.
Labels: -ReleaseBlock-Dev
Issue 878574 has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30

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

commit a475e1abdde98c7315fd567687935576ac47ca4a
Author: Clark DuVall <cduvall@chromium.org>
Date: Thu Aug 30 20:44:06 2018

Fix crash in web request proxies connection error handler

When WebRequestAPI is destroyed, it posts a task to the IO thread to
shutdown the proxies. This crash can happen when the WebRequestAPI is
destroyed, and a connection error happens on one of the proxies (which
accesses data destroyed by the profile) before the ProxySet can be fully
shutdown on the IO thread. This change adds an atomic flag to check to
make sure the WebRequestAPI has not been destroyed before running error
handlers.

Bug:  878366 
Change-Id: I2f7b98c1d6df7c53a5917444ac996e9b5ef4e794
Reviewed-on: https://chromium-review.googlesource.com/1196031
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587749}
[modify] https://crrev.com/a475e1abdde98c7315fd567687935576ac47ca4a/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/a475e1abdde98c7315fd567687935576ac47ca4a/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/a475e1abdde98c7315fd567687935576ac47ca4a/extensions/browser/api/web_request/web_request_api.h
[modify] https://crrev.com/a475e1abdde98c7315fd567687935576ac47ca4a/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/a475e1abdde98c7315fd567687935576ac47ca4a/extensions/browser/api/web_request/web_request_proxying_websocket.cc

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 31

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

commit 63affad9eb485822ced25a7333acc20e36aefdd3
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Aug 31 01:37:29 2018

Revert "Fix crash in web request proxies connection error handler"

This reverts commit a475e1abdde98c7315fd567687935576ac47ca4a.

Reason for revert: Test failing on Win debug and msan.

Original change's description:
> Fix crash in web request proxies connection error handler
> 
> When WebRequestAPI is destroyed, it posts a task to the IO thread to
> shutdown the proxies. This crash can happen when the WebRequestAPI is
> destroyed, and a connection error happens on one of the proxies (which
> accesses data destroyed by the profile) before the ProxySet can be fully
> shutdown on the IO thread. This change adds an atomic flag to check to
> make sure the WebRequestAPI has not been destroyed before running error
> handlers.
> 
> Bug:  878366 
> Change-Id: I2f7b98c1d6df7c53a5917444ac996e9b5ef4e794
> Reviewed-on: https://chromium-review.googlesource.com/1196031
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#587749}

TBR=rockot@chromium.org,cduvall@chromium.org

Change-Id: I5825fdb5449001062095bd56ed39eb8b76e055e3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  878366 
Reviewed-on: https://chromium-review.googlesource.com/1198488
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587902}
[modify] https://crrev.com/63affad9eb485822ced25a7333acc20e36aefdd3/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/63affad9eb485822ced25a7333acc20e36aefdd3/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/63affad9eb485822ced25a7333acc20e36aefdd3/extensions/browser/api/web_request/web_request_api.h
[modify] https://crrev.com/63affad9eb485822ced25a7333acc20e36aefdd3/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/63affad9eb485822ced25a7333acc20e36aefdd3/extensions/browser/api/web_request/web_request_proxying_websocket.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 31

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

commit eb70882071cfaea93b0126a724cf0c135076f1c6
Author: Clark DuVall <cduvall@chromium.org>
Date: Fri Aug 31 03:09:02 2018

Reland "Fix crash in web request proxies connection error handler"

This is a reland of a475e1abdde98c7315fd567687935576ac47ca4a

Test needed to call Shutdown() so WebRequestAPI is cleaned up properly.

Original change's description:
> Fix crash in web request proxies connection error handler
>
> When WebRequestAPI is destroyed, it posts a task to the IO thread to
> shutdown the proxies. This crash can happen when the WebRequestAPI is
> destroyed, and a connection error happens on one of the proxies (which
> accesses data destroyed by the profile) before the ProxySet can be fully
> shutdown on the IO thread. This change adds an atomic flag to check to
> make sure the WebRequestAPI has not been destroyed before running error
> handlers.
>
> Bug:  878366 
> Change-Id: I2f7b98c1d6df7c53a5917444ac996e9b5ef4e794
> Reviewed-on: https://chromium-review.googlesource.com/1196031
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#587749}

TBR=rockot@chromium.org

Bug:  878366 
Change-Id: I97ab12e12fe4955af94138ec5bb64c1391d27598
Reviewed-on: https://chromium-review.googlesource.com/1198567
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587927}
[modify] https://crrev.com/eb70882071cfaea93b0126a724cf0c135076f1c6/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/eb70882071cfaea93b0126a724cf0c135076f1c6/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/eb70882071cfaea93b0126a724cf0c135076f1c6/extensions/browser/api/web_request/web_request_api.h
[modify] https://crrev.com/eb70882071cfaea93b0126a724cf0c135076f1c6/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/eb70882071cfaea93b0126a724cf0c135076f1c6/extensions/browser/api/web_request/web_request_proxying_websocket.cc

Fix for this bug is in review at http://crrev.com/c/1205470
 Issue 880114  has been merged into this issue.
Cc: wfh@chromium.org
if you're interested, I have a reliable repro for this in  issue 880114 . I can verify after lands, if you want.
Thanks for the repro, it looks like the WebRequestProxyingWebSocket crash is actually different than the WebRequestProxyingURLLoader crash. I'm going to reopen the original bug on that crash (issue 878574), and put up a fix for that one too.
Status: Fixed (was: Assigned)
Looks like the WebRequestProxyingURLLoaderFactory crash is no longer showing up in 71.0.3543.0 canary, I'm going to close this. The WebRequestProxyingWebSocket crash is tracked in issue 878574.
Labels: Merge-Request-70 M-70
clark, let's get this into M70 branch.
Project Member

Comment 16 by sheriffbot@chromium.org, Sep 5

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Looks like bugdroid hasn't posted a comment yet, but this merge request is for http://crrev.com/c/1205470.
Labels: -Merge-Review-70 Merge-Approved-70
approved branch:3538
This was merged into M70 in http://crrev.com/c/1207354 if bugdroid doesn't comment again.
Labels: CommitLog-Audit-Violation Merge-Without-Approval
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 61a92ca6b14054280304985768ff4032a304f33c was merged to refs/branch-heads/3538 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
Labels: -CommitLog-Audit-Violation
Project Member

Comment 22 by bugdroid1@chromium.org, Sep 5

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

commit 86bc694455e1e754512434f1d44a4f3fc84a7821
Author: Clark DuVall <cduvall@chromium.org>
Date: Tue Sep 04 23:47:28 2018

Fix web request proxy crash by moving ownership to ResourceContext

The web request proxies will now be owned completely on the IO thread by
ResourceContext. This should prevent any handlers from being run after
the ResourceContext is destroyed.

Bug:  878366 
Change-Id: I629d81597ee3ab3835a63ebe47cb97fa4497b36a
Reviewed-on: https://chromium-review.googlesource.com/1205470
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588692}
[modify] https://crrev.com/86bc694455e1e754512434f1d44a4f3fc84a7821/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/86bc694455e1e754512434f1d44a4f3fc84a7821/extensions/browser/api/web_request/web_request_api.h
[modify] https://crrev.com/86bc694455e1e754512434f1d44a4f3fc84a7821/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/86bc694455e1e754512434f1d44a4f3fc84a7821/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.h
[modify] https://crrev.com/86bc694455e1e754512434f1d44a4f3fc84a7821/extensions/browser/api/web_request/web_request_proxying_websocket.cc
[modify] https://crrev.com/86bc694455e1e754512434f1d44a4f3fc84a7821/extensions/browser/api/web_request/web_request_proxying_websocket.h

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 5

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61a92ca6b14054280304985768ff4032a304f33c

commit 61a92ca6b14054280304985768ff4032a304f33c
Author: Clark DuVall <cduvall@chromium.org>
Date: Wed Sep 05 16:43:31 2018

Fix web request proxy crash by moving ownership to ResourceContext

The web request proxies will now be owned completely on the IO thread by
ResourceContext. This should prevent any handlers from being run after
the ResourceContext is destroyed.

Bug:  878366 
Change-Id: I629d81597ee3ab3835a63ebe47cb97fa4497b36a
Reviewed-on: https://chromium-review.googlesource.com/1205470
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588692}(cherry picked from commit 86bc694455e1e754512434f1d44a4f3fc84a7821)
Reviewed-on: https://chromium-review.googlesource.com/1207354
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#50}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/61a92ca6b14054280304985768ff4032a304f33c/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/61a92ca6b14054280304985768ff4032a304f33c/extensions/browser/api/web_request/web_request_api.h
[modify] https://crrev.com/61a92ca6b14054280304985768ff4032a304f33c/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc
[modify] https://crrev.com/61a92ca6b14054280304985768ff4032a304f33c/extensions/browser/api/web_request/web_request_proxying_url_loader_factory.h
[modify] https://crrev.com/61a92ca6b14054280304985768ff4032a304f33c/extensions/browser/api/web_request/web_request_proxying_websocket.cc
[modify] https://crrev.com/61a92ca6b14054280304985768ff4032a304f33c/extensions/browser/api/web_request/web_request_proxying_websocket.h

Labels: -Merge-Without-Approval

Sign in to add a comment