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

Issue 762601 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Cronet iOS: Eliminate race condition when CronetEnironment is destructed

Project Member Reported by kapishnikov@chromium.org, Sep 6 2017

Issue description

In the CronetEnvironment destructor, we post a task to execute PrepareForDestroyOnNetworkThread to network thread and wait until all network thread tasks are completed. There is a chance that between posting the task and waiting for all tasks to complete, somebody will post another task that relies on CronetEnvironment members being alive. As the result, the PrepareForDestroyOnNetworkThread will complete and invalidate the member variables; and the task that follows will crash.

CronetEnvironment::~CronetEnvironment() {
  PostToNetworkThread(
      FROM_HERE,
      base::Bind(&CronetEnvironment::PrepareForDestroyOnNetworkThread,
                 base::Unretained(this)));
  // Some
  if (network_io_thread_) {
    // Deleting a thread blocks the current thread and waits until all pending
    // tasks are completed.
    network_io_thread_.reset(nullptr);
  }
}
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 8 2017

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

commit 263338811aeaaa96e0967ed0e5f00d6902020405
Author: kapishnikov <kapishnikov@chromium.org>
Date: Fri Sep 08 14:25:56 2017

Cronet iOS: Eliminate race condition when CronetEnironment is destructed

While CronetEnvironment is being destructed, another thread can post a task to the network thread,
a task that may try to use destructed members. Therefore, it should be guaranteed that there are no tasks
other tasks posted after execution of CleanUpOnNetworkThread() (previously PrepareForDestroyOnNetworkThread())
This is avoided by performing cleanup in Thread::Cleanup() so no future tasks will be executed.

BUG= 762601 

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Iaa30d4395822252f6fefceaeb2d95c254ab0b9b2
Reviewed-on: https://chromium-review.googlesource.com/653617
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Andrei Kapishnikov <kapishnikov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500587}
[modify] https://crrev.com/263338811aeaaa96e0967ed0e5f00d6902020405/components/cronet/ios/cronet_environment.h
[modify] https://crrev.com/263338811aeaaa96e0967ed0e5f00d6902020405/components/cronet/ios/cronet_environment.mm

Status: Fixed (was: Assigned)

Sign in to add a comment