MB-23987: Ignore repeatedly scheduled tasks 35/77035/9
authorJames Harrison <00jamesh@gmail.com>
Thu, 20 Apr 2017 09:49:38 +0000 (10:49 +0100)
committerDave Rigby <daver@couchbase.com>
Mon, 24 Apr 2017 16:52:43 +0000 (16:52 +0000)
commit3f816b0995fc7919da3d556144d303ba4cfd9a52
treeecf900a148426586f606c11db9bef33ecaa6fd93
parent96f2d6c9cd628078e92aa6543f01da629e511e2e
MB-23987: Ignore repeatedly scheduled tasks

A timeout was observed in a test when running against an ephemeral
bucket.

This arose during tear down from a deadlock between
ExecutorPool::_unregisterTaskable and ExecutorPool::_cancel.

Normally, this should not be possible, as _unregisterTaskable waits for
the task locator to be empty after cancelling each of the tasks,
indicating that all tasks have been taken up by a thead and cleaned up.

The problem developed from the ItemPager as used in ephemeral buckets.
KVBucket::enableItemPager cancels and schedules the same task object.
Cancelling a task simply marks it as dead; it must later be cleaned up
by a ExecutorThread.

ExecutorPool::_schedule was altered to reset dead tasks to running to
facilitate enableItemPager. Therefore, if a thread did not clean up the
task between the cancel and schedule, enableItemPager reduced down to
just a schedule. If the task was already scheduled, this would be a
duplicate schedule.

Duplicate scheduling does not adversely affect the taskLocator as it is
a map. However, the taskQueues /will/ have a second copy of the task
added.

Now, when unregisterTaskable finds the taskLocator to be empty, it is
still possible for a thread to take up the duplicate copy of the task.
The task is already marked dead, and so the thread immediately calls
_cancel to clean it up.

if _unregisterTaskable already holds the mutex, _cancel will block.
_unregisterTaskable will then join all the ExecutorThreads, deadlocking
when it reaches the thread blocked on _cancel.

Change-Id: Ia0dc7e8e8c3423e1098c71cf376653b23b4393e6
Reviewed-on: http://review.couchbase.org/77035
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
src/executorpool.cc
src/fakes/fake_executorpool.h
tests/module_tests/executorpool_test.cc
tests/module_tests/executorpool_test.h