Potential deadlocks in bigtable::MutationBatcher

[coryan]

Does this issue affect the google-cloud-cpp project?

Yes.

What component of google-cloud-cpp is this related to?

Bigtable

Describe the bug

This code is suspect:

google-cloud-cpp/google/cloud/bigtable/mutation_batcher.cc

Lines 185 to 189 in a68f42a

	std::unique_lock<std::mutex> lk(mu_);
	outstanding_size_ -= batch.requests_size;
	num_requests_pending_ -= num_mutations;
	num_outstanding_batches_--;
	SatisfyPromises(TryAdmit(cq), lk); // unlocks the lock

It calls TryAdmin() which performs a lot of work, potentially making RPCs:

google-cloud-cpp/google/cloud/bigtable/mutation_batcher.cc

Lines 192 to 207 in a68f42a

	std::vector<MutationBatcher::AdmissionPromise> MutationBatcher::TryAdmit(
	CompletionQueue& cq) {
	// Defer satisfying promises until we release the lock.
	std::vector<AdmissionPromise> admission_promises;
	do {
	while (!pending_mutations_.empty() &&
	HasSpaceFor(pending_mutations_.front())) {
	auto& mut = pending_mutations_.front();
	admission_promises.emplace_back(std::move(mut.admission_promise));
	Admit(std::move(mut));
	pending_mutations_.pop();
	}
	} while (FlushIfPossible(cq));
	return admission_promises;
	}

This can deadlock and/or make no progress.

To Reproduce

This is a report from a customer, in their application they need to run about a thousand processes for the deadlock to happen.

Expected behavior

The MutationBatcher should guarantee progress.

Operating system:

Linux, no further details available.

[coryan]

There is a bug in CompletionQueue::RunAsync() which still causes problems with this.