New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 770451 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Access violation in RenderViewHostTestHarness destructor

Project Member Reported by xhw...@chromium.org, Sep 30 2017

Issue description

During RenderViewHostTestHarness destruction process, when destroying a frame, ~RenderFrameHostImpl will call content::MockRenderProcessHost::RemoveRoute(int) [1]. However, MockRenderProcessHost was already destructed, as part of RenderViewHostTestEnabler destruction process [2].

This is because the RenderViewHostTestEnabler member was declared after the WebContents member [3], so it's destructed before WebContents is destructed.

To fix this, we can probably switch the declaration member, which seems to work for me. But honest I know very little how these works.

[1] Crash stack

  000d29f7  base::internal::LockImpl::Lock()
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/base/synchronization/lock_impl_posix.cc:63
  00081973  base::Lock::Acquire()
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/base/synchronization/lock.h:45
  v------>  AutoLock                                                                                                                                                                                                                                                                                                                        /usr/local/google/workspace/clank/src/base/synchronization/lock.h:115
  000c6dd9  base::SequenceCheckerImpl::CalledOnValidSequence() const                                                                                                                                                                                                                                                                        /usr/local/google/workspace/clank/src/base/sequence_checker_impl.cc:58
  01656513  base::IDMap<IPC::Listener*, int>::Lookup(int) const                                                                                                                                                                                                                                                                             /usr/local/google/workspace/clank/src/base/containers/id_map.h:118

  016564c1  content::MockRenderProcessHost::RemoveRoute(int)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  008d58ff  ~RenderFrameHostImpl
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/content/browser/frame_host/render_frame_host_impl.cc:603
  016729c7  ~TestRenderFrameHost
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/content/test/test_render_frame_host.cc:75
  v------>  std::__ndk1::default_delete<content::RenderFrameHostImpl>::operator()(content::RenderFrameHostImpl*) const                                                                                                                                                          
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2598
  008e7fd1  ~RenderFrameHostManager
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/content/browser/frame_host/render_frame_host_manager.cc:84
  008be39b  ~FrameTreeNode
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/content/browser/frame_host/frame_tree_node.cc:194
  008bd0eb  ~FrameTree
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/content/browser/frame_host/frame_tree.cc:116
  00a87699  ~WebContentsImpl
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/content/browser/web_contents/web_contents_impl.cc:677
  016743b7  ~TestWebContents
                                                                                                                                                          /usr/local/google/worksp
ace/clank/src/content/test/test_web_contents.cc:52
  016743ff  ~TestWebContents
                                                                                                                                                          
  01665b9b  ~RenderViewHostTestHarness

[2] MockRenderProcessHost destruction stack

~MockRenderProcessHost
~NotifyingMockRenderProcessHost
~MockRenderProcessHostFactory                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                            ~RenderViewHostTestEnabler                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   ~RenderViewHostTestHarness                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   ~MediaDrmStorageImplTest_RemoveSession_InvalidSession_Test 

[3] https://cs.chromium.org/chromium/src/content/public/test/test_renderer_host.h?rcl=fca18931b5ee2b1e272ba0e86ac98f4c10d12e05&l=265
 
Cc: nick@chromium.org
Switching the order of members to fix dtor ordering issues sounds fine to me (we have a lot of classes where order of destruction of the members is significant)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 3 2017

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

commit eb13e97790b3be01be36bc586f54ee0bca3ce157
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Oct 03 22:48:46 2017

content: Fix RenderViewHostTestHarness destruction order

Declare |rvh_test_enabler_| before |contents_| so that it will be
destructed after |contents_|. Otherwise, when |contents_| is destructed,
it could cause access violation. See BUG for details.

BUG= 770451 

Change-Id: I10a6924c7fa166c9cc9a8cd3943121e2422f5498
Reviewed-on: https://chromium-review.googlesource.com/696984
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506215}
[modify] https://crrev.com/eb13e97790b3be01be36bc586f54ee0bca3ce157/content/public/test/test_renderer_host.h

Status: Fixed (was: Available)

Sign in to add a comment