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

Issue 743232 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Race in Cronet initialization enables HTTP cache when it shouldn't be enabled

Project Member Reported by mge...@chromium.org, Jul 14 2017

Issue description

There's a race between creation of the CronetURLRequestAdapter and initialization of the CronetURLRequestContextAdapter on the network thread that causes cache to be enabled when it shouldn't be if a UrlRequest is created immediately after building the CronetEngine.
 

Comment 1 by mge...@chromium.org, Jul 14 2017

To clarify, this is about the LOAD_DISABLE_CACHE load flag. So if I'm understanding this correctly, it's mostly a bug in the behavior of HTTP_CACHE_DISK_NO_HTTP, but has some effects on the HostCache and maybe other things I don't know about in the case of HTTP_CACHE_DISABLED.

Comment 2 by mge...@chromium.org, Jul 17 2017

Owner: mge...@chromium.org
Status: Assigned (was: Untriaged)
Yikes CronetURLRequestAdapter() reads from CronetURLRequestContextAdapter::default_load_flags_...which isn't initialized until CronetURLRequestContextAdapter::InitializeOnNetworkThread()

Comment 4 by mge...@chromium.org, Jul 17 2017

Yup, that's what I found. If not for this bug, testHostCachePersistence would have failed every time instead of about 1% of the time. I'm fixing it now.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 17 2017

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

commit 5b7eb518eef51dfbc0fbc05330dfba8994863f92
Author: Miriam Gershenson <mgersh@chromium.org>
Date: Mon Jul 17 20:34:04 2017

Fix Cronet load flag initialization

Load flags were being read from the CronetURLRequestContextAdapter
on the main thread, which could potentially happen before they were
initialized on the network thread if a UrlRequest is created immediately
after the CronetEngine. Now they are read at request start time on the
network thread, which must be after context initialization.

BUG= 743232 

Change-Id: I3f9c5cd1cde906555c5ba68b4a45a9a3ee71b014
Reviewed-on: https://chromium-review.googlesource.com/574827
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Miriam Gershenson <mgersh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487227}
[modify] https://crrev.com/5b7eb518eef51dfbc0fbc05330dfba8994863f92/components/cronet/android/cronet_url_request_adapter.cc
[modify] https://crrev.com/5b7eb518eef51dfbc0fbc05330dfba8994863f92/components/cronet/android/cronet_url_request_context_adapter.cc
[modify] https://crrev.com/5b7eb518eef51dfbc0fbc05330dfba8994863f92/components/cronet/android/cronet_url_request_context_adapter.h
[modify] https://crrev.com/5b7eb518eef51dfbc0fbc05330dfba8994863f92/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java

Comment 6 by mge...@chromium.org, Jul 17 2017

Status: Fixed (was: Assigned)

Sign in to add a comment