[usb][mediatek] Fix thread teardown race.
Currently, zx-primitives are being used to synchronize the halting of a
processing thread. This is racy if the containing object is being
deallocated. This replaces the primitives with a call to thrd_join(),
which fixes the race. This seems to only affect clang-x64 builds.
FLK-244 #done
Test: runtests -t mt-hci-endpoint-test-test -r 1000 (~3% racy before)
Change-Id: Ifa2f58f876d117868d4b2ea7fc0ebc0a52d3dd7a
diff --git a/zircon/system/dev/usb/mt-musb-host/usb-device.cpp b/zircon/system/dev/usb/mt-musb-host/usb-device.cpp
index b582e66..5cf0028 100644
--- a/zircon/system/dev/usb/mt-musb-host/usb-device.cpp
+++ b/zircon/system/dev/usb/mt-musb-host/usb-device.cpp
@@ -65,7 +65,6 @@
for (uint8_t i=0; i<=kMaxEpNum; i++) {
if (ep_[i] != nullptr) {
ep_[i]->Halt();
- ep_[i]->Join();
}
}
}
diff --git a/zircon/system/dev/usb/mt-musb-host/usb-endpoint-test.cpp b/zircon/system/dev/usb/mt-musb-host/usb-endpoint-test.cpp
index 76cf71c..6f0349b 100644
--- a/zircon/system/dev/usb/mt-musb-host/usb-endpoint-test.cpp
+++ b/zircon/system/dev/usb/mt-musb-host/usb-endpoint-test.cpp
@@ -102,7 +102,6 @@
TestingEndpoint te(mmio_->View(0));
EXPECT_OK(te.StartQueueThread());
EXPECT_OK(te.Halt());
- te.Join();
}
TEST_F(TransactionEndpointTest, QueueThread_Enqueue) {
@@ -118,7 +117,6 @@
te.Wait();
EXPECT_OK(te.Halt());
- te.Join();
te.m_dispatch().VerifyAndClear();
}
@@ -158,7 +156,6 @@
te.Wait(5);
EXPECT_OK(te.Halt());
- te.Join();
te.m_dispatch().VerifyAndClear();
}
@@ -198,7 +195,6 @@
te.Wait(5);
EXPECT_OK(te.Halt());
- te.Join();
te.m_dispatch().VerifyAndClear();
}
@@ -238,7 +234,6 @@
te.Wait(5);
EXPECT_OK(te.Halt());
- te.Join();
te.m_dispatch().VerifyAndClear();
}
diff --git a/zircon/system/dev/usb/mt-musb-host/usb-endpoint.cpp b/zircon/system/dev/usb/mt-musb-host/usb-endpoint.cpp
index a8a9caa..b7c759a 100644
--- a/zircon/system/dev/usb/mt-musb-host/usb-endpoint.cpp
+++ b/zircon/system/dev/usb/mt-musb-host/usb-endpoint.cpp
@@ -57,13 +57,27 @@
}
zx_status_t TransactionEndpoint::Halt() {
- fbl::AutoLock _(&pending_lock_);
- if (transaction_) {
- transaction_->Cancel();
+ {
+ fbl::AutoLock _(&pending_lock_);
+ if (transaction_) {
+ transaction_->Cancel();
+ }
+
+ halted_ = true;
+ pending_cond_.Signal();
}
- halted_ = true;
- pending_cond_.Signal();
+ if (pending_thread_) {
+ int rc;
+ int status = thrd_join(pending_thread_, &rc);
+ if (status != thrd_success) {
+ zxlogf(ERROR, "could not join pending_thread\n");
+ return ZX_ERR_INTERNAL;
+ } else if (rc != 0) {
+ zxlogf(ERROR, "pending_thread returned nonzero status: %d\n", rc);
+ return ZX_ERR_INTERNAL;
+ }
+ }
return ZX_OK;
}
@@ -77,7 +91,6 @@
fbl::AutoLock _(&pending_lock_);
if (pending_.is_empty()) {
if (halted_.load()) {
- sync_completion_signal(&complete_);
return 0;
}
@@ -88,7 +101,6 @@
pending_cond_.Wait(&pending_lock_);
if (halted_.load()) {
- sync_completion_signal(&complete_);
return 0;
}
}
diff --git a/zircon/system/dev/usb/mt-musb-host/usb-endpoint.h b/zircon/system/dev/usb/mt-musb-host/usb-endpoint.h
index 6a46207..9dbb7d95 100644
--- a/zircon/system/dev/usb/mt-musb-host/usb-endpoint.h
+++ b/zircon/system/dev/usb/mt-musb-host/usb-endpoint.h
@@ -48,9 +48,6 @@
// Halt endpoint request processing. All outstanding requests will result in a
// ZX_ERR_IO_NOT_PRESENT status, and the queue thread will be shut down.
virtual zx_status_t Halt() = 0;
-
- // Block and wait until this endpoint is fully halted.
- virtual void Join() = 0;
};
// A TransactionEndpoint is an Endpoint which dispatches requests to a Transaction for processing.
@@ -72,7 +69,6 @@
zx_status_t CancelAll() override;
size_t GetMaxTransferSize() override ;
zx_status_t Halt() override;
- void Join() override { __UNUSED auto _ = sync_completion_wait(&complete_, ZX_TIME_INFINITE); }
protected:
// The USB register mmio.
@@ -111,9 +107,6 @@
// Queue dispatch condition and associated mutex.
fbl::Mutex pending_lock_;
fbl::ConditionVariable pending_cond_ TA_GUARDED(pending_lock_);
-
- // A completion that is signaled upon completing the halt process.
- sync_completion_t complete_;
};
// A ControlEndpoint is an Endpoint dispatching control-type transactions.
