New issue
Advanced search Search tips

Issue 616476 link

Starred by 1 user
Status: Fixed
Owner:
boliu@chromium.org
Closed: Jun 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Null pointer in nativeSetWebKitSharedTimersSuspended

Project Member Reported by boliu@chromium.org, Jun 1 2016 
RenderProcessHostImpl::DecrementWorkerRefCount needs a !run_renderer_in_process guard, just like RemoveRoute.

* The crashing line is "host->RemoveObserver(this);" and since the fault address is 0x0, pretty safe to assume |host| is null.
* It's the first use of the pointer after obtaining it from RenderProcessHost::FromID. This means that host has been removed from g_all_hosts, but neither RenderProcessExited nor RenderProcessHostDestroyed has been called on the RPHObserver.
* RegisterHost is called from RPH constructor. Nothing interesting
* UnregisterHost is called from destructor (fail safe I guess), and Cleanup.
* Cleanup is called from DecrementWorkerRefCount and RemoveRoute. From here, kinda obvious that DecrementWorkerRefCount is missing the guard
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 1 2016 

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

commit 55cb5a389f401a539225c7e7b59197114db5cd29
Author: boliu <boliu@chromium.org>
Date: Wed Jun 01 20:44:33 2016

Avoid deleting RenderProcessHostImpl for in-process

This is suspected to be the cause of null pointer crash in Android
WebView when RenderProcessHostImpl::FromID unexpectedly returns null.

Issue is RenderProcessHostImpl::DecrementWorkerRefCount does not
check for run_renderer_in_process() like RemoveRoute. Move the check
to beginning of Cleanup instead.

BUG= 616476 

Review-Url: https://codereview.chromium.org/2029623002
Cr-Commit-Position: refs/heads/master@{#397235}

[modify] https://crrev.com/55cb5a389f401a539225c7e7b59197114db5cd29/content/browser/renderer_host/render_process_host_impl.cc

Comment 2 by boliu@chromium.org, Jun 2 2016

Labels: Merge-Request-52

Comment 3 by tin...@google.com, Jun 2 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 2 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53b10adeda96e325d3c1ebdb81bf5a370400999d

commit 53b10adeda96e325d3c1ebdb81bf5a370400999d
Author: Bo Liu <boliu@chromium.org>
Date: Thu Jun 02 15:35:42 2016

[Merge M52] Avoid deleting RenderProcessHostImpl for in-process

This is suspected to be the cause of null pointer crash in Android
WebView when RenderProcessHostImpl::FromID unexpectedly returns null.

Issue is RenderProcessHostImpl::DecrementWorkerRefCount does not
check for run_renderer_in_process() like RemoveRoute. Move the check
to beginning of Cleanup instead.

BUG= 616476 

Review-Url: https://codereview.chromium.org/2029623002
Cr-Commit-Position: refs/heads/master@{#397235}
(cherry picked from commit 55cb5a389f401a539225c7e7b59197114db5cd29)

Review URL: https://codereview.chromium.org/2036743002 .

Cr-Commit-Position: refs/branch-heads/2743@{#189}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/53b10adeda96e325d3c1ebdb81bf5a370400999d/content/browser/renderer_host/render_process_host_impl.cc

Comment 5 by boliu@chromium.org, Jun 2 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment