Fix deadlock from outside Perfetto mutex
Fixes the deadlock described by the below two thread states: Thread 1: - ConsumeAll() (takes mBufferMutex) - perfetto::DataSource::Trace() (takes Perfetto mutex) Thread 2: - StartDataSourceImpl() (takes Perfetto mutex) - NetworkTracePoller::Start() (takes mBufferMutex) Locks should always be taken in the same order. To achieve this, ConsumeAll only locks mBufferMutex while accessing the buffer, and releases it before calling the callback (which under typical use calls DataSource::Trace() as in "Thread 1"). As a fun side note, Start() always calls ConsumeAll(), so there exists a lock sequence of perfetto mutex -> mBufferMutex -> perfetto mutex, thus this order was always "exercised". However, Perfetto uses a recursive mutex, so the second lock was a no-op, making this hard to catch. Bug: 320700806 Test: atest libnetworkstats_test Change-Id: If8c5e0e218666ec53a287ec56333a6d3b8cfe08f
This commit is contained in:
parent
7e9f27be7d
commit
e8a769f2c3
1 changed files with 13 additions and 11 deletions
|
|
@ -149,18 +149,20 @@ void NetworkTracePoller::TraceIfaces(const std::vector<PacketTrace>& packets) {
|
|||
}
|
||||
|
||||
bool NetworkTracePoller::ConsumeAll() {
|
||||
std::scoped_lock<std::mutex> lock(mBufferMutex);
|
||||
if (mRingBuffer == nullptr) {
|
||||
ALOGW("Tracing is not active");
|
||||
return false;
|
||||
}
|
||||
|
||||
std::vector<PacketTrace> packets;
|
||||
base::Result<int> ret = mRingBuffer->ConsumeAll(
|
||||
[&](const PacketTrace& pkt) { packets.push_back(pkt); });
|
||||
if (!ret.ok()) {
|
||||
ALOGW("Failed to poll ringbuf: %s", ret.error().message().c_str());
|
||||
return false;
|
||||
{
|
||||
std::scoped_lock<std::mutex> lock(mBufferMutex);
|
||||
if (mRingBuffer == nullptr) {
|
||||
ALOGW("Tracing is not active");
|
||||
return false;
|
||||
}
|
||||
|
||||
base::Result<int> ret = mRingBuffer->ConsumeAll(
|
||||
[&](const PacketTrace& pkt) { packets.push_back(pkt); });
|
||||
if (!ret.ok()) {
|
||||
ALOGW("Failed to poll ringbuf: %s", ret.error().message().c_str());
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
ATRACE_INT("NetworkTracePackets", packets.size());
|
||||
|
|
|
|||
Loading…
Reference in a new issue