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

Issue 757113 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Use-after-destruction in QuicProxyClientSocketTest

Project Member Reported by brucedaw...@chromium.org, Aug 18 2017

Issue description

Chrome Version: Local build, R#495306
OS: Windows

What steps will reproduce the problem?
(1) Set is_clang = false
(2) Build net_unittests.exe
(3) net_unittests --gtest_filter=Version/QuicProxyClientSocketTest.AsyncWriteAroundReads/6
(4) Note pure-virt call crash

This happens because QuicProxyClientSocketTest contains an embedded MockClock object, named clock_. The address of that clock is passed to the std::unique_ptr<QuicProxyClientSocket> sock_ constructor. However clock_ is declared later in the class that sock_, so clock_ is destroyed and is then used in the Disconnect() call from inside ~QuicProxyClientSocket. The fix is to move the clock_ declaration up above the sock_ declaration.

This bug is not found by clang, presumably indicating that it does not reset v-table pointers in all cases or does not use pure-virt pointers.

 
Status: Started (was: Assigned)
Trivial patch upcoming
The CL that should fix this is in crrev.com/c/622327. However to really tell that it's fixed you need to monitor crrev.com/c/614920, since that's what found the bug in the first place.
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 19 2017

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

commit 8fb43d0a60b05c974e32ca4375f3cbe217a4ed43
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Sat Aug 19 01:12:42 2017

Fix QuicProxyClientSocketTest destruction order bug

In QuicProxyClientSocketTest the clock_ object was passed to the sock_
object but was destroyed earlier (due to being declared later in the
class). This change fixes that by moving the clock_ object earlier.

The bug was found during VC++ testing because it triggered pure-virtual
calls which are fatal errors in VC++ builds.

TBR=rch@chromium.org
BUG= 757113 

Change-Id: I9e5d930a8d3586feee2da0d5fa0f9926b67e09b6
Reviewed-on: https://chromium-review.googlesource.com/622327
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495776}
[modify] https://crrev.com/8fb43d0a60b05c974e32ca4375f3cbe217a4ed43/net/quic/chromium/quic_proxy_client_socket_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment