Fix protocol for aborting and waiting for cancelable tasks.

Fix protocol for aborting and waiting for cancelable tasks.

Since {CancelAndWait} blocks on the tasks that are still present in the internal
hashmap, we are not allowed to remove the task upon trying to cancel it using
{TryAbort}.

The previous implementation suffered from a bug where:
1) The task was created and handed over to the platform.
2) The task was started by the platform, setting it to running state.
3) We called {TryAbort}, effectively removing it from the manager, but failing
   to cancel (as it was already running)
4) All tasks finished running, indicating this with their own semaphore.
5) The platform was stuck (scheduling) before destroying the task.
6) Main thread finished its work, waiting for all the necessary tasks, and the
   isolate terminated.
7) The platform destroyed the task, calling the destructor, calling into an
   already freed isolate.

BUG=chromium:524425
LOG=N

Committed: https://crrev.com/a698fd849bba5df74a44b31e284ec45362f33c12
Cr-Commit-Position: refs/heads/master@{#32118}

[Michael Lippautz, 2015-11-19 14:05:34]

Description was changed from

==========
Fix protocol for aborting and waiting for cancelable tasks.

Since {CancelAndWait} blocks on the tasks that are still present in the internal
hashmap, we are not allowed to remove the task upon trying to cancel it using
{TryAbort}.

The previous implementation suffered from a bug where:
1) The task was created and handed over to the platform.
2) We called {TryAbort}, effectively removing it from the manager.
2a) Cancel failed since the task was already running.
3) We waited for all the running tasks (fine)
4) All tasks finished running, indicating this with their own semaphore.
5a) The platform was stuck (scheduling) before destroying the task.
6) Main thread finished and the isolate terminated.
7) The platform destroyed the task, calling the destructor, calling into an
   already freed isolate.

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

to

==========
Fix protocol for aborting and waiting for cancelable tasks.

Since {CancelAndWait} blocks on the tasks that are still present in the internal
hashmap, we are not allowed to remove the task upon trying to cancel it using
{TryAbort}.

The previous implementation suffered from a bug where:
1) The task was created and handed over to the platform.
2) We called {TryAbort}, effectively removing it from the manager.
2a) Cancel failed since the task was already running.
3) We waited for all the running tasks (fine)
4) All tasks finished running, indicating this with their own semaphore.
5a) The platform was stuck (scheduling) before destroying the task.
6) Main thread finished and the isolate terminated.
7) The platform destroyed the task, calling the destructor, calling into an
   already freed isolate.

BUG=chromium:524425
LOG=N
==========

[Hannes Payer (out of office), 2015-11-19 14:19:06]

hpayer@chromium.org changed reviewers:
+ hpayer@chromium.org

[Hannes Payer (out of office), 2015-11-19 14:19:07]

lgtm

https://codereview.chromium.org/1460763004/diff/1/src/cancelable-task.cc
File src/cancelable-task.cc (right):

https://codereview.chromium.org/1460763004/diff/1/src/cancelable-task.cc#newc...
src/cancelable-task.cc:57: CHECK(removed != nullptr);
DCHECK

https://codereview.chromium.org/1460763004/diff/1/src/cancelable-task.cc#newc...
src/cancelable-task.cc:72: CHECK(removed != nullptr);
DCHECK

[Michael Lippautz, 2015-11-19 14:20:30]

https://codereview.chromium.org/1460763004/diff/1/src/cancelable-task.cc
File src/cancelable-task.cc (right):

https://codereview.chromium.org/1460763004/diff/1/src/cancelable-task.cc#newc...
src/cancelable-task.cc:57: CHECK(removed != nullptr);
On 2015/11/19 14:19:07, Hannes Payer wrote:
> DCHECK

Done.

https://codereview.chromium.org/1460763004/diff/1/src/cancelable-task.cc#newc...
src/cancelable-task.cc:72: CHECK(removed != nullptr);
On 2015/11/19 14:19:07, Hannes Payer wrote:
> DCHECK

Done.

[Michael Lippautz, 2015-11-19 14:20:41]

The CQ bit was checked by mlippautz@chromium.org

[Michael Lippautz, 2015-11-19 14:20:41]

The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org
Link to the patchset: https://codereview.chromium.org/1460763004/#ps20001
(title: "Addressed comments")

[commit-bot: I haz the power, 2015-11-19 14:20:47]

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

[Michael Lippautz, 2015-11-19 14:26:29]

Description was changed from

==========
Fix protocol for aborting and waiting for cancelable tasks.

Since {CancelAndWait} blocks on the tasks that are still present in the internal
hashmap, we are not allowed to remove the task upon trying to cancel it using
{TryAbort}.

The previous implementation suffered from a bug where:
1) The task was created and handed over to the platform.
2) The task was started by the platform, setting it to running state.
2) We called {TryAbort}, effectively removing it from the manager, but failing
   to cancel (as it was already running)
4) All tasks finished running, indicating this with their own semaphore.
5) The platform was stuck (scheduling) before destroying the task.
6) Main thread finished its work, waiting for all the necessary tasks, and the
   isolate terminated.
7) The platform destroyed the task, calling the destructor, calling into an
   already freed isolate.

BUG=chromium:524425
LOG=N
==========

to

==========
Fix protocol for aborting and waiting for cancelable tasks.

Since {CancelAndWait} blocks on the tasks that are still present in the internal
hashmap, we are not allowed to remove the task upon trying to cancel it using
{TryAbort}.

The previous implementation suffered from a bug where:
1) The task was created and handed over to the platform.
2) The task was started by the platform, setting it to running state.
3) We called {TryAbort}, effectively removing it from the manager, but failing
   to cancel (as it was already running)
4) All tasks finished running, indicating this with their own semaphore.
5) The platform was stuck (scheduling) before destroying the task.
6) Main thread finished its work, waiting for all the necessary tasks, and the
   isolate terminated.
7) The platform destroyed the task, calling the destructor, calling into an
   already freed isolate.

BUG=chromium:524425
LOG=N
==========

[commit-bot: I haz the power, 2015-11-19 15:33:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-19 15:33:59]

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