Do not delete synchronously in OnTaskRunnerDeleter.

Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, thread-unsafe objects can not be safely deleted  under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behavior.

Review-Url: https://codereview.chromium.org/2657283002
Cr-Commit-Position: refs/heads/master@{#446920}
Committed: https://chromium.googlesource.com/chromium/src/+/90cdf584494922804c8d0c81a83bcad271645659

[pkalinnikov, 2017-01-27 15:16:45]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

BUG=
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it cannot safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
from a thread to the task runner to access the Object. Meanwhile, the deleter
has been invoked and immediately deleted the Object. After that a task in the
TaskRunner got to execute and tries to read from a deleted Object.
==========

[pkalinnikov, 2017-01-27 15:19:33]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it cannot safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
from a thread to the task runner to access the Object. Meanwhile, the deleter
has been invoked and immediately deleted the Object. After that a task in the
TaskRunner got to execute and tries to read from a deleted Object.
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
from a thread to the task runner to access the Object. After that, the deleter
has been invoked and immediately deleted the Object. After that the Task got to
execute in the TaskRunner, and tries to read from a deleted Object => Crash.
==========

[pkalinnikov, 2017-01-27 15:21:26]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
from a thread to the task runner to access the Object. After that, the deleter
has been invoked and immediately deleted the Object. After that the Task got to
execute in the TaskRunner, and tries to read from a deleted Object => Crash.
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
from a to a TaskRunner to access the Object. After that, the deleter has been
invoked and immediately deleted the Object. After that the Task got to execute
in the TaskRunner, and tries to read from a deleted Object => Crash.
==========

[pkalinnikov, 2017-01-27 15:22:42]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
from a to a TaskRunner to access the Object. After that, the deleter has been
invoked and immediately deleted the Object. After that the Task got to execute
in the TaskRunner, and tries to read from a deleted Object => Crash.
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behaviour.
==========

[pkalinnikov, 2017-01-27 15:27:25]

pkalinnikov@chromium.org changed reviewers:
+ dcheng@chromium.org, tzik@chromium.org

[pkalinnikov, 2017-01-27 15:27:26]

Hi guys, please take a look.

tzik@: You are the author of the class, and apparently so far the only user of
it.
dcheng@: You reviewed the CL introduced this class.

[pkalinnikov, 2017-01-27 16:32:29]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has a weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behaviour.
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behaviour.
==========

[pkalinnikov, 2017-01-27 16:32:58]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely return thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behaviour.
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely delete thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behaviour.
==========

[dcheng, 2017-01-27 19:25:09]

Is this causing a bug somewhere? I'm OK with this change, but it seems largely
theoretical?

[pkalinnikov, 2017-01-27 19:27:45]

I have a pending CL that relies on fixing this behavior. I could make such a
class in our component locally, but fixing the existing one seems the right way
to go.

[tzik, 2017-01-27 21:04:01]

lgtm

[dcheng, 2017-01-28 01:26:10]

LGTM.

Originally I was wondering why we had the synchronous delete, but if tzik is OK
with this, then I am too.

[pkalinnikov, 2017-01-28 09:55:22]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, it can not safely delete thread-unsafe objects under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behaviour.
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, thread-unsafe objects can not be safely deleted  under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behavior.
==========

[pkalinnikov, 2017-01-28 09:55:29]

The CQ bit was checked by pkalinnikov@chromium.org

[commit-bot: I haz the power, 2017-01-28 09:55:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-28 11:40:27]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1485597329542100, "parent_rev":
"b2f66c7a95b2d366e8972efc5acac53d60f4978d", "commit_rev":
"90cdf584494922804c8d0c81a83bcad271645659"}

[commit-bot: I haz the power, 2017-01-28 11:41:03]

Description was changed from

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, thread-unsafe objects can not be safely deleted  under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behavior.
==========

to

==========
Do not delete synchronously in OnTaskRunnerDeleter.

Currently OnTaskRunnerDeleter::operator() relies on RunsTasksOnCurrentThread()
method of the TaskRunner. However, the latter has weak guarantees on the
returned value (see the comment to it). It can even always return true.
Therefore, thread-unsafe objects can not be safely deleted  under some
circumstances.

The current implementation also makes such a crashing scenario possible. Suppose
there is an Object living in the SequencedTaskRunner and a Task has been posted
to a TaskRunner to access the Object. After that, the deleter has been invoked
and immediately deleted the Object. After that the Task got to execute in the
TaskRunner, and tries to read from a deleted Object => undefined behavior.

Review-Url: https://codereview.chromium.org/2657283002
Cr-Commit-Position: refs/heads/master@{#446920}
Committed:
https://chromium.googlesource.com/chromium/src/+/90cdf584494922804c8d0c81a83b...
==========

[commit-bot: I haz the power, 2017-01-28 11:41:04]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/90cdf584494922804c8d0c81a83b...