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

Issue 845942 link

Starred by 1 user
Status: Fixed
Owner:
mmenke@chromium.org
Closed: May 2018
Cc:
█ xunji...@chromium.org
jam@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification



Sign in to add a comment

NetworkService: NetworkContext lifetime code is incorrect

Project Member Reported by mmenke@chromium.org, May 23 2018 
NetworkContexts that have their |network_service_| field populated delete themselves when their mojo pipe is closed.  This is incorrect for NetworkContexts not created in the network context (i.e., those that just wrap a URLRequestContext or those created with a URLRequestContextBuilder), both types of which are owned by their constructor, not the NetworkService.

While this is, somewhat strangely, not causing crashes, we should still clean it up.

Comment 1 by mmenke@chromium.org, May 23 2018

Status: Fixed (was: Started)
Oops, linked the wrong CL on the fix.  This has landed:

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

commit cfc40b9d0aeffc727aaa9f3366b03a915041ae5e
Author: Matt Menke <mmenke@chromium.org>
Date: Wed May 23 21:54:19 2018

NetworkService: Refactor NetworkContext ownership.

NetworkContexts used to delete themselves when their mojo pipe was
closed. This was only correct behavior for NetworkContexts that live in
the out-of-process NetworkService.  Other NetworkContexts are stored in
unique_ptrs owned by the IOThread, ProfileIOData, and the
StoragePartition's NetworkContext class, so should not delete themselves.

We haven't been running into crashes due to this, but it seems less than
ideal. This CL changes things so that the NetworkService actually owns
NetworkContexts that need to be destroyed on pipe closures, and the
NetworkContexts call into it to tell the NetworkService to delete them
when the pipe is closed.  This mirrors the URLLoaderFactory ownership
model.

Bug:  845939 
Change-Id: Ie53d8b6114000b0f403839089d6ee6dec1d29932
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1070427
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561265}
[modify] https://crrev.com/cfc40b9d0aeffc727aaa9f3366b03a915041ae5e/services/network/http_cache_data_remover_unittest.cc
[modify] https://crrev.com/cfc40b9d0aeffc727aaa9f3366b03a915041ae5e/services/network/network_context.cc
[modify] https://crrev.com/cfc40b9d0aeffc727aaa9f3366b03a915041ae5e/services/network/network_context.h
[modify] https://crrev.com/cfc40b9d0aeffc727aaa9f3366b03a915041ae5e/services/network/network_context_unittest.cc
[modify] https://crrev.com/cfc40b9d0aeffc727aaa9f3366b03a915041ae5e/services/network/network_service.cc
[modify] https://crrev.com/cfc40b9d0aeffc727aaa9f3366b03a915041ae5e/services/network/network_service.h
►

Sign in to add a comment