Add move-only lambdas support
Bug: 166800618
Test: atest --host android.hardware.biometrics.fingerprint.WorkerThreadTest
Change-Id: I582d44d5098b7426663b75200c822bc6e8bb70a6
diff --git a/biometrics/fingerprint/aidl/default/WorkerThread.cpp b/biometrics/fingerprint/aidl/default/WorkerThread.cpp
index 512efb8..d1a63d0 100644
--- a/biometrics/fingerprint/aidl/default/WorkerThread.cpp
+++ b/biometrics/fingerprint/aidl/default/WorkerThread.cpp
@@ -36,7 +36,7 @@
mThread.join();
}
-bool WorkerThread::schedule(Task&& task) {
+bool WorkerThread::schedule(std::unique_ptr<Callable> task) {
if (mIsDestructing) {
return false;
}
@@ -58,10 +58,10 @@
if (mIsDestructing) {
return;
}
- Task task = std::move(mQueue.front());
+ std::unique_ptr<Callable> task = std::move(mQueue.front());
mQueue.pop_front();
lock.unlock();
- task();
+ (*task)();
}
}
diff --git a/biometrics/fingerprint/aidl/default/include/Callable.h b/biometrics/fingerprint/aidl/default/include/Callable.h
new file mode 100644
index 0000000..c629511
--- /dev/null
+++ b/biometrics/fingerprint/aidl/default/include/Callable.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+namespace aidl::android::hardware::biometrics::fingerprint {
+
+// Interface for representing parameterless functions. Unlike std::function<void()>, this can also
+// represent move-only lambdas.
+class Callable {
+ public:
+ virtual void operator()() = 0;
+ virtual ~Callable() = default;
+
+ // Creates a heap-allocated Callable instance from any function object.
+ template <typename T>
+ static std::unique_ptr<Callable> from(T func);
+
+ private:
+ template <typename T>
+ class AnyFuncWrapper;
+};
+
+// Private helper class for wrapping any function object into a Callable.
+template <typename T>
+class Callable::AnyFuncWrapper : public Callable {
+ public:
+ explicit AnyFuncWrapper(T func) : mFunc(std::move(func)) {}
+
+ void operator()() override { mFunc(); }
+
+ private:
+ T mFunc;
+};
+
+template <typename T>
+std::unique_ptr<Callable> Callable::from(T func) {
+ return std::make_unique<AnyFuncWrapper<T>>(std::move(func));
+}
+
+} // namespace aidl::android::hardware::biometrics::fingerprint
\ No newline at end of file
diff --git a/biometrics/fingerprint/aidl/default/include/WorkerThread.h b/biometrics/fingerprint/aidl/default/include/WorkerThread.h
index 49104c8..6fff4f2 100644
--- a/biometrics/fingerprint/aidl/default/include/WorkerThread.h
+++ b/biometrics/fingerprint/aidl/default/include/WorkerThread.h
@@ -21,9 +21,9 @@
#include <queue>
#include <thread>
-namespace aidl::android::hardware::biometrics::fingerprint {
+#include "Callable.h"
-using Task = std::function<void()>;
+namespace aidl::android::hardware::biometrics::fingerprint {
// A class that encapsulates a worker thread and a task queue, and provides a convenient interface
// for a Session to schedule its tasks for asynchronous execution.
@@ -47,7 +47,12 @@
// If the internal queue is not full, pushes a task at the end of the queue and returns true.
// Otherwise, returns false. If the queue is busy, blocks until it becomes available.
- bool schedule(Task&& task);
+ // This method expects heap-allocated tasks because it's the simplest way to represent function
+ // objects of any type. Stack-allocated std::function could be used instead, but it cannot
+ // represent functions with move-only captures because std::function is inherently copyable.
+ // Not being able to pass move-only lambdas is a major limitation for the HAL implementation,
+ // so heap-allocated tasks that share a common interface (Callable) were chosen instead.
+ bool schedule(std::unique_ptr<Callable> task);
private:
// The function that runs on the internal thread. Sequentially runs the available tasks from
@@ -63,7 +68,7 @@
std::atomic<bool> mIsDestructing;
// Queue that's guarded by mQueueMutex and mQueueCond.
- std::deque<Task> mQueue;
+ std::deque<std::unique_ptr<Callable>> mQueue;
std::mutex mQueueMutex;
std::condition_variable mQueueCond;
diff --git a/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp b/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp
index ba901ad..0d5014bb 100644
--- a/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp
+++ b/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp
@@ -25,13 +25,14 @@
namespace {
+using aidl::android::hardware::biometrics::fingerprint::Callable;
using aidl::android::hardware::biometrics::fingerprint::WorkerThread;
using namespace std::chrono_literals;
TEST(WorkerThreadTest, ScheduleReturnsTrueWhenQueueHasSpace) {
WorkerThread worker(1 /*maxQueueSize*/);
for (int i = 0; i < 100; ++i) {
- EXPECT_TRUE(worker.schedule([] {}));
+ EXPECT_TRUE(worker.schedule(Callable::from([] {})));
// Allow enough time for the previous task to be processed.
std::this_thread::sleep_for(2ms);
}
@@ -40,16 +41,16 @@
TEST(WorkerThreadTest, ScheduleReturnsFalseWhenQueueIsFull) {
WorkerThread worker(2 /*maxQueueSize*/);
// Add a long-running task.
- worker.schedule([] { std::this_thread::sleep_for(1s); });
+ worker.schedule(Callable::from([] { std::this_thread::sleep_for(1s); }));
// Allow enough time for the worker to start working on the previous task.
std::this_thread::sleep_for(2ms);
// Fill the worker's queue to the maximum.
- worker.schedule([] {});
- worker.schedule([] {});
+ worker.schedule(Callable::from([] {}));
+ worker.schedule(Callable::from([] {}));
- EXPECT_FALSE(worker.schedule([] {}));
+ EXPECT_FALSE(worker.schedule(Callable::from([] {})));
}
TEST(WorkerThreadTest, TasksExecuteInOrder) {
@@ -58,19 +59,19 @@
std::vector<int> results;
for (int i = 0; i < NUM_TASKS; ++i) {
- worker.schedule([&results, i] {
+ worker.schedule(Callable::from([&results, i] {
// Delay tasks differently to provoke races.
std::this_thread::sleep_for(std::chrono::nanoseconds(100 - i % 100));
// Unguarded write to results to provoke races.
results.push_back(i);
- });
+ }));
}
std::promise<void> promise;
auto future = promise.get_future();
// Schedule a special task to signal when all of the tasks are finished.
- worker.schedule([&promise] { promise.set_value(); });
+ worker.schedule(Callable::from([&promise] { promise.set_value(); }));
auto status = future.wait_for(1s);
ASSERT_EQ(status, std::future_status::ready);
@@ -86,11 +87,11 @@
{
WorkerThread worker(2 /*maxQueueSize*/);
- worker.schedule([&promise1] {
+ worker.schedule(Callable::from([&promise1] {
promise1.set_value();
std::this_thread::sleep_for(200ms);
- });
- worker.schedule([&promise2] { promise2.set_value(); });
+ }));
+ worker.schedule(Callable::from([&promise2] { promise2.set_value(); }));
// Make sure the first task is executing.
auto status1 = future1.wait_for(1s);