Fix bug in the way an Event handles a bound thread.
This just keeps the (test) Event representation in sync with
the runtime Event representation, which is modified by
https://googleplex-android-review.git.corp.google.com/#/c/platform/frameworks/ml/+/2880348/
Bug: 63905942
Test: vts
Change-Id: I2a6de4397c5e31e35cb3d02e241dd21452c21ca6
diff --git a/neuralnetworks/1.0/vts/functional/Event.cpp b/neuralnetworks/1.0/vts/functional/Event.cpp
index 0fab86b..67de4f5 100644
--- a/neuralnetworks/1.0/vts/functional/Event.cpp
+++ b/neuralnetworks/1.0/vts/functional/Event.cpp
@@ -10,9 +10,15 @@
Event::Event() : mStatus(Status::WAITING) {}
Event::~Event() {
- if (mThread.joinable()) {
- mThread.join();
- }
+ // Note that we cannot call Event::join_thread from here: Event is
+ // intended to be reference counted, and it is possible that the
+ // reference count drops to zero in the bound thread, causing the
+ // bound thread to call this destructor. If a thread tries to join
+ // itself, it throws an exception, producing a message like the
+ // following:
+ //
+ // terminating with uncaught exception of type std::__1::system_error:
+ // thread::join failed: Resource deadlock would occur
}
Return<void> Event::notify(ReturnedStatus status) {
@@ -38,6 +44,7 @@
Event::Status Event::wait() {
std::unique_lock<std::mutex> lock(mMutex);
mCondition.wait(lock, [this]{return mStatus != Status::WAITING;});
+ join_thread_locked();
return mStatus;
}
@@ -69,6 +76,17 @@
return true;
}
+void Event::join_thread() {
+ std::lock_guard<std::mutex> lock(mMutex);
+ join_thread_locked();
+}
+
+void Event::join_thread_locked() {
+ if (mThread.joinable()) {
+ mThread.join();
+ }
+}
+
} // namespace implementation
} // namespace V1_0
} // namespace neuralnetworks
diff --git a/neuralnetworks/1.0/vts/functional/Event.h b/neuralnetworks/1.0/vts/functional/Event.h
index 2e19585..4f7f2f6 100644
--- a/neuralnetworks/1.0/vts/functional/Event.h
+++ b/neuralnetworks/1.0/vts/functional/Event.h
@@ -136,12 +136,21 @@
* TODO: What if notify has already been called before on_finish?
* TODO: Why does the return value of the callback matter?
*/
- bool on_finish(std::function<bool(void)> callback);
+ bool on_finish(std::function<bool(void)> callback);
/**
- * Event::bind_thread binds a thread to the event ensuring that the thread
- * has fully finished and cleaned its resources before the event is
- * destroyed. The thread should be bound using std::move.
+ * Event::bind_thread binds a thread to the event for later use by
+ * Event::join_thread.
+ *
+ * The thread must be passed using std::move.
+ *
+ * Once a thread is bound with Event::bind_thread, the client code
+ * should ensure that one of the following occurs before the event is
+ * destroyed:
+ * - Event::join_thread has been called.
+ * - Event::wait has been called.
+ * - Event::wait_for has been called and returned other than TIMEOUT.
+ * - Event::wait_until has been called and returned other than TIMEOUT.
*
* The bound thread shall not call any Event method with the exception of
* IEvent::notify, which it will call when the thread has finished its
@@ -154,9 +163,20 @@
* asyncThread.joinable() must be true.
* @return bool True if successful, false if thread was not properly bound.
*/
- bool bind_thread(std::thread&& asyncThread);
+ bool bind_thread(std::thread&& asyncThread);
+
+ /**
+ * Event::join_thread ensures that the thread (if any) bound to
+ * this event with Event::bind_thread has fully finished and
+ * cleaned its resources. It is legal to call this function
+ * multiple times, concurrently or sequentially.
+ */
+ void join_thread();
private:
+ // Same as Event::join_thread but assumes we already hold a lock on mMutex.
+ void join_thread_locked();
+
Status mStatus;
std::mutex mMutex;
std::condition_variable mCondition;
@@ -172,6 +192,9 @@
std::unique_lock<std::mutex> lock(mMutex);
std::cv_status status = mCondition.wait_for(lock, timeout_duration,
[this]{return mStatus != Status::WAITING;});
+ if (status != std::cv_status::timeout) {
+ join_thread_locked();
+ }
return status != std::cv_status::timeout ? mStatus : Status::TIMEOUT;
}
@@ -180,6 +203,9 @@
std::unique_lock<std::mutex> lock(mMutex);
std::cv_status status = mCondition.wait_until(lock, timeout_time,
[this]{return mStatus != Status::WAITING;});
+ if (status != std::cv_status::timeout) {
+ join_thread_locked();
+ }
return status != std::cv_status::timeout ? mStatus : Status::TIMEOUT;
}