Fix use-after-free on Linux at shutdown

Fix use-after-free on Linux at shutdown

The address tracker has a file-description watcher that references
a libevent event that is deleted when the message-loop is deleted.

Fixed by deleting the address-tracker before the message-loop.

BUG=595316
R=derekjchow@chromium.org

Committed: https://crrev.com/143cc3e1332fe8ff4c9cac86fe07187c2cad3f75
Cr-Commit-Position: refs/heads/master@{#382271}

[derekjchow1, 2016-03-16 17:12:25]

derekjchow@chromium.org changed reviewers:
+ pauljensen@chromium.org

[derekjchow1, 2016-03-16 17:12:25]

+pauljensen for OWNERS.

This looks like a reasonable change but can you post the Valgrind output as
well? I believe nothing should be trying to access NetworkChangeNotifier after
the thread has shutdown.

[pauljensen, 2016-03-17 13:42:49]

looks good, but, Allan, I see you've signed the CLA, can you add yourself to
src/AUTHORS in this CL also please?

[allan.jensen, 2016-03-17 15:04:41]

On 2016/03/17 at 13:42:49, pauljensen wrote:
> looks good, but, Allan, I see you've signed the CLA, can you add yourself to
src/AUTHORS in this CL also please?

Another patch that added me to AUTHORS should have gone in today.

[allan.jensen, 2016-03-18 08:48:22]

On 2016/03/17 at 15:04:41, allan.jensen wrote:
> On 2016/03/17 at 13:42:49, pauljensen wrote:
> > looks good, but, Allan, I see you've signed the CLA, can you add yourself to
src/AUTHORS in this CL also please?
> 
> Another patch that added me to AUTHORS should have gone in today.

I posted the valgrind trace in the bug. This specific trace happens using a
special configuration using a newer libevent, but even using the older version
Chromium uses, the behaviour is still undefined, but not detected by valgrind.

[pauljensen, 2016-03-18 18:27:49]

https://codereview.chromium.org/1808543003/diff/1/net/base/network_change_not...
File net/base/network_change_notifier_linux.cc (right):

https://codereview.chromium.org/1808543003/diff/1/net/base/network_change_not...
net/base/network_change_notifier_linux.cc:72: address_tracker_.reset();
Can you add a comment like:
// Delete AddressTrackerLinux before MessageLoop gets deleted as
// AddressTrackerLinux's FileDescriptorWatcher holds a pointer to the
// MessageLoop.

[allan.jensen, 2016-03-19 12:56:49]

On 2016/03/18 at 18:27:49, pauljensen wrote:
>
https://codereview.chromium.org/1808543003/diff/1/net/base/network_change_not...
> File net/base/network_change_notifier_linux.cc (right):
> 
>
https://codereview.chromium.org/1808543003/diff/1/net/base/network_change_not...
> net/base/network_change_notifier_linux.cc:72: address_tracker_.reset();
> Can you add a comment like:
> // Delete AddressTrackerLinux before MessageLoop gets deleted as
> // AddressTrackerLinux's FileDescriptorWatcher holds a pointer to the
> // MessageLoop.

Good idea

[pauljensen, 2016-03-21 12:04:21]

lgtm

[pauljensen, 2016-03-21 12:04:25]

The CQ bit was checked by pauljensen@chromium.org

[commit-bot: I haz the power, 2016-03-21 12:04:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1808543003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1808543003/20001

[commit-bot: I haz the power, 2016-03-21 13:00:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-21 13:02:10]

Description was changed from

==========
Fix use-after-free on Linux at shutdown

The address tracker has a file-description watcher that references
a libevent event that is deleted when the message-loop is deleted.

Fixed by deleting the address-tracker before the message-loop.

BUG=595316
R=derekjchow@chromium.org
==========

to

==========
Fix use-after-free on Linux at shutdown

The address tracker has a file-description watcher that references
a libevent event that is deleted when the message-loop is deleted.

Fixed by deleting the address-tracker before the message-loop.

BUG=595316
R=derekjchow@chromium.org

Committed: https://crrev.com/143cc3e1332fe8ff4c9cac86fe07187c2cad3f75
Cr-Commit-Position: refs/heads/master@{#382271}
==========

[commit-bot: I haz the power, 2016-03-21 13:02:11]

Patchset 2 (id:??) landed as
https://crrev.com/143cc3e1332fe8ff4c9cac86fe07187c2cad3f75
Cr-Commit-Position: refs/heads/master@{#382271}