GitLab

SchedulerWorker can touch SchedulerWorkerPoolImpl
through SchedulerWorker::delegate_->outer_.

This could cause use-after-free in tests (not a problem in prod because
we never join workers nor destroy the scheduler/pools) because
SchedulerWorkerPoolImpl no longer accounts for a SchedulerWorker
after invoking SchedulerWorker::Cleanup and hence will not join it
in JoinForTesting() before it is itself destroyed.

Before this CL: a SchedulerWorker could access SchedulerWorkerPoolImpl
after Cleanup() and hence potentially use after free if the pool is
destroyed before the detached thread completes execution.

This CL reinforces JoinForTesting() to wait for all workers that
ever had a reference to |this| (i.e. its |workers_| + any recently
reclaimed workers).

Added TaskSchedulerWorkerPoolTest.RacyCleanup as regression test although
it passes in most scenarios without the fix because this race is hard to
exercise...

And added a DCHECK on start that the requested pool size isn't greater than
kMaxNumberOfWorkers (as I tried to add 1000 worker in my new test and had to
debug why it hung until I rediscovered this intentional behavior).

Bug: 810464
Change-Id: Ibaefb5446d462c325e4ddfd34a54ad3c448c1417
Reviewed-on: https://chromium-review.googlesource.com/931547
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539095}