Merge changes Ic84efbde,I7c1c6044 into sc-dev
* changes:
Implement a simple worker thread
Move headers to include/ and clean up style
diff --git a/biometrics/fingerprint/aidl/default/Android.bp b/biometrics/fingerprint/aidl/default/Android.bp
index c8cb663..24087cf 100644
--- a/biometrics/fingerprint/aidl/default/Android.bp
+++ b/biometrics/fingerprint/aidl/default/Android.bp
@@ -1,18 +1,32 @@
cc_binary {
name: "android.hardware.biometrics.fingerprint-service.example",
+ vendor: true,
relative_install_path: "hw",
init_rc: ["fingerprint-default.rc"],
vintf_fragments: ["fingerprint-default.xml"],
- vendor: true,
+ local_include_dirs: ["include"],
+ srcs: [
+ "main.cpp",
+ "Fingerprint.cpp",
+ "Session.cpp",
+ ],
shared_libs: [
"libbase",
"libbinder_ndk",
"android.hardware.biometrics.fingerprint-V1-ndk_platform",
"android.hardware.biometrics.common-V1-ndk_platform",
],
+}
+
+cc_test_host {
+ name: "android.hardware.biometrics.fingerprint.WorkerThreadTest",
+ local_include_dirs: ["include"],
srcs: [
- "main.cpp",
- "Fingerprint.cpp",
- "Session.cpp",
+ "tests/WorkerThreadTest.cpp",
+ "WorkerThread.cpp",
],
+ shared_libs: [
+ "libcutils",
+ ],
+ test_suites: ["general-tests"],
}
diff --git a/biometrics/fingerprint/aidl/default/Fingerprint.cpp b/biometrics/fingerprint/aidl/default/Fingerprint.cpp
index f27e278..fa3171f 100644
--- a/biometrics/fingerprint/aidl/default/Fingerprint.cpp
+++ b/biometrics/fingerprint/aidl/default/Fingerprint.cpp
@@ -18,50 +18,44 @@
#include "Session.h"
namespace aidl::android::hardware::biometrics::fingerprint {
+namespace {
-const int kSensorId = 1;
-const common::SensorStrength kSensorStrength = common::SensorStrength::STRONG;
-const int kMaxEnrollmentsPerUser = 5;
-const FingerprintSensorType kSensorType = FingerprintSensorType::REAR;
-const bool kSupportsNavigationGestures = true;
-const std::string kHwDeviceName = "fingerprintSensor";
-const std::string kHardwareVersion = "vendor/model/revision";
-const std::string kFirmwareVersion = "1.01";
-const std::string kSerialNumber = "00000001";
+constexpr int SENSOR_ID = 1;
+constexpr common::SensorStrength SENSOR_STRENGTH = common::SensorStrength::STRONG;
+constexpr int MAX_ENROLLMENTS_PER_USER = 5;
+constexpr FingerprintSensorType SENSOR_TYPE = FingerprintSensorType::REAR;
+constexpr bool SUPPORTS_NAVIGATION_GESTURES = true;
+constexpr char HW_DEVICE_NAME[] = "fingerprintSensor";
+constexpr char HW_VERSION[] = "vendor/model/revision";
+constexpr char FW_VERSION[] = "1.01";
+constexpr char SERIAL_NUMBER[] = "00000001";
-ndk::ScopedAStatus Fingerprint::getSensorProps(std::vector<SensorProps>* return_val) {
- *return_val = std::vector<SensorProps>();
+} // namespace
- std::vector<common::HardwareInfo> hardwareInfos = std::vector<common::HardwareInfo>();
- common::HardwareInfo sensorInfo = {kHwDeviceName,
- kHardwareVersion,
- kFirmwareVersion,
- kSerialNumber
- };
- hardwareInfos.push_back(sensorInfo);
- common::CommonProps commonProps = {kSensorId,
- kSensorStrength,
- kMaxEnrollmentsPerUser,
- hardwareInfos};
- SensorLocation sensorLocation = {
- 0 /* displayId */,
- 0 /* sensorLocationX */,
- 0 /* sensorLocationY */,
- 0 /* sensorRadius */
- };
- SensorProps props = {commonProps,
- kSensorType,
- {sensorLocation},
- kSupportsNavigationGestures,
- false /* supportsDetectInteraction */};
- return_val->push_back(props);
+Fingerprint::Fingerprint() {}
+
+ndk::ScopedAStatus Fingerprint::getSensorProps(std::vector<SensorProps>* out) {
+ std::vector<common::HardwareInfo> hardwareInfos = {
+ {HW_DEVICE_NAME, HW_VERSION, FW_VERSION, SERIAL_NUMBER}};
+
+ common::CommonProps commonProps = {SENSOR_ID, SENSOR_STRENGTH, MAX_ENROLLMENTS_PER_USER,
+ hardwareInfos};
+
+ SensorLocation sensorLocation = {0 /* displayId */, 0 /* sensorLocationX */,
+ 0 /* sensorLocationY */, 0 /* sensorRadius */};
+
+ *out = {{commonProps,
+ SENSOR_TYPE,
+ {sensorLocation},
+ SUPPORTS_NAVIGATION_GESTURES,
+ false /* supportsDetectInteraction */}};
return ndk::ScopedAStatus::ok();
}
ndk::ScopedAStatus Fingerprint::createSession(int32_t /*sensorId*/, int32_t /*userId*/,
const std::shared_ptr<ISessionCallback>& cb,
- std::shared_ptr<ISession>* return_val) {
- *return_val = SharedRefBase::make<Session>(cb);
+ std::shared_ptr<ISession>* out) {
+ *out = SharedRefBase::make<Session>(cb);
return ndk::ScopedAStatus::ok();
}
} // namespace aidl::android::hardware::biometrics::fingerprint
diff --git a/biometrics/fingerprint/aidl/default/Session.cpp b/biometrics/fingerprint/aidl/default/Session.cpp
index 8446221..52dddb6 100644
--- a/biometrics/fingerprint/aidl/default/Session.cpp
+++ b/biometrics/fingerprint/aidl/default/Session.cpp
@@ -26,7 +26,7 @@
ndk::ScopedAStatus cancel() override { return ndk::ScopedAStatus::ok(); }
};
-Session::Session(std::shared_ptr<ISessionCallback> cb) : cb_(std::move(cb)) {}
+Session::Session(std::shared_ptr<ISessionCallback> cb) : mCb(std::move(cb)) {}
ndk::ScopedAStatus Session::generateChallenge(int32_t /*cookie*/, int32_t /*timeoutSec*/) {
LOG(INFO) << "generateChallenge";
@@ -39,32 +39,32 @@
}
ndk::ScopedAStatus Session::enroll(int32_t /*cookie*/, const keymaster::HardwareAuthToken& /*hat*/,
- std::shared_ptr<common::ICancellationSignal>* /*return_val*/) {
+ std::shared_ptr<common::ICancellationSignal>* /*out*/) {
LOG(INFO) << "enroll";
return ndk::ScopedAStatus::ok();
}
ndk::ScopedAStatus Session::authenticate(int32_t /*cookie*/, int64_t /*keystoreOperationId*/,
- std::shared_ptr<common::ICancellationSignal>* return_val) {
+ std::shared_ptr<common::ICancellationSignal>* out) {
LOG(INFO) << "authenticate";
- if (cb_) {
- cb_->onStateChanged(0, SessionState::AUTHENTICATING);
+ if (mCb) {
+ mCb->onStateChanged(0, SessionState::AUTHENTICATING);
}
- *return_val = SharedRefBase::make<CancellationSignal>();
+ *out = SharedRefBase::make<CancellationSignal>();
return ndk::ScopedAStatus::ok();
}
ndk::ScopedAStatus Session::detectInteraction(
- int32_t /*cookie*/, std::shared_ptr<common::ICancellationSignal>* /*return_val*/) {
+ int32_t /*cookie*/, std::shared_ptr<common::ICancellationSignal>* /*out*/) {
LOG(INFO) << "detectInteraction";
return ndk::ScopedAStatus::ok();
}
ndk::ScopedAStatus Session::enumerateEnrollments(int32_t /*cookie*/) {
LOG(INFO) << "enumerateEnrollments";
- if (cb_) {
- cb_->onStateChanged(0, SessionState::ENUMERATING_ENROLLMENTS);
- cb_->onEnrollmentsEnumerated(std::vector<int32_t>());
+ if (mCb) {
+ mCb->onStateChanged(0, SessionState::ENUMERATING_ENROLLMENTS);
+ mCb->onEnrollmentsEnumerated(std::vector<int32_t>());
}
return ndk::ScopedAStatus::ok();
}
@@ -72,18 +72,18 @@
ndk::ScopedAStatus Session::removeEnrollments(int32_t /*cookie*/,
const std::vector<int32_t>& /*enrollmentIds*/) {
LOG(INFO) << "removeEnrollments";
- if (cb_) {
- cb_->onStateChanged(0, SessionState::REMOVING_ENROLLMENTS);
- cb_->onEnrollmentsRemoved(std::vector<int32_t>());
+ if (mCb) {
+ mCb->onStateChanged(0, SessionState::REMOVING_ENROLLMENTS);
+ mCb->onEnrollmentsRemoved(std::vector<int32_t>());
}
return ndk::ScopedAStatus::ok();
}
ndk::ScopedAStatus Session::getAuthenticatorId(int32_t /*cookie*/) {
LOG(INFO) << "getAuthenticatorId";
- if (cb_) {
- cb_->onStateChanged(0, SessionState::GETTING_AUTHENTICATOR_ID);
- cb_->onAuthenticatorIdRetrieved(0 /* authenticatorId */);
+ if (mCb) {
+ mCb->onStateChanged(0, SessionState::GETTING_AUTHENTICATOR_ID);
+ mCb->onAuthenticatorIdRetrieved(0 /* authenticatorId */);
}
return ndk::ScopedAStatus::ok();
}
diff --git a/biometrics/fingerprint/aidl/default/WorkerThread.cpp b/biometrics/fingerprint/aidl/default/WorkerThread.cpp
new file mode 100644
index 0000000..512efb8
--- /dev/null
+++ b/biometrics/fingerprint/aidl/default/WorkerThread.cpp
@@ -0,0 +1,68 @@
+/*
+ * 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.
+ */
+
+#include "WorkerThread.h"
+
+namespace aidl::android::hardware::biometrics::fingerprint {
+
+// It's important that mThread is initialized after everything else because it runs a member
+// function that may use any member of this class.
+WorkerThread::WorkerThread(size_t maxQueueSize)
+ : mMaxSize(maxQueueSize),
+ mIsDestructing(false),
+ mQueue(),
+ mQueueMutex(),
+ mQueueCond(),
+ mThread(&WorkerThread::threadFunc, this) {}
+
+WorkerThread::~WorkerThread() {
+ // This is a signal for threadFunc to terminate as soon as possible, and a hint for schedule
+ // that it doesn't need to do any work.
+ mIsDestructing = true;
+ mQueueCond.notify_all();
+ mThread.join();
+}
+
+bool WorkerThread::schedule(Task&& task) {
+ if (mIsDestructing) {
+ return false;
+ }
+
+ std::unique_lock<std::mutex> lock(mQueueMutex);
+ if (mQueue.size() >= mMaxSize) {
+ return false;
+ }
+ mQueue.push_back(std::move(task));
+ lock.unlock();
+ mQueueCond.notify_one();
+ return true;
+}
+
+void WorkerThread::threadFunc() {
+ while (!mIsDestructing) {
+ std::unique_lock<std::mutex> lock(mQueueMutex);
+ mQueueCond.wait(lock, [this] { return !mQueue.empty() || mIsDestructing; });
+ if (mIsDestructing) {
+ return;
+ }
+ Task task = std::move(mQueue.front());
+ mQueue.pop_front();
+ lock.unlock();
+ task();
+ }
+}
+
+} // namespace aidl::android::hardware::biometrics::fingerprint
diff --git a/biometrics/fingerprint/aidl/default/Fingerprint.h b/biometrics/fingerprint/aidl/default/include/Fingerprint.h
similarity index 91%
rename from biometrics/fingerprint/aidl/default/Fingerprint.h
rename to biometrics/fingerprint/aidl/default/include/Fingerprint.h
index 4e952ba..867e5fa 100644
--- a/biometrics/fingerprint/aidl/default/Fingerprint.h
+++ b/biometrics/fingerprint/aidl/default/include/Fingerprint.h
@@ -20,13 +20,15 @@
namespace aidl::android::hardware::biometrics::fingerprint {
-class Fingerprint : public BnFingerprint {
+class Fingerprint final : public BnFingerprint {
public:
- ndk::ScopedAStatus getSensorProps(std::vector<SensorProps>* _aidl_return) override;
+ Fingerprint();
+
+ ndk::ScopedAStatus getSensorProps(std::vector<SensorProps>* out) override;
ndk::ScopedAStatus createSession(int32_t sensorId, int32_t userId,
const std::shared_ptr<ISessionCallback>& cb,
- std::shared_ptr<ISession>* _aidl_return) override;
+ std::shared_ptr<ISession>* out) override;
};
} // namespace aidl::android::hardware::biometrics::fingerprint
diff --git a/biometrics/fingerprint/aidl/default/Session.h b/biometrics/fingerprint/aidl/default/include/Session.h
similarity index 89%
rename from biometrics/fingerprint/aidl/default/Session.h
rename to biometrics/fingerprint/aidl/default/include/Session.h
index ed3ae3f..b9befef 100644
--- a/biometrics/fingerprint/aidl/default/Session.h
+++ b/biometrics/fingerprint/aidl/default/include/Session.h
@@ -33,14 +33,13 @@
ndk::ScopedAStatus revokeChallenge(int32_t cookie, int64_t challenge) override;
ndk::ScopedAStatus enroll(int32_t cookie, const keymaster::HardwareAuthToken& hat,
- std::shared_ptr<common::ICancellationSignal>* return_val) override;
+ std::shared_ptr<common::ICancellationSignal>* out) override;
- ndk::ScopedAStatus authenticate(
- int32_t cookie, int64_t keystoreOperationId,
- std::shared_ptr<common::ICancellationSignal>* return_val) override;
+ ndk::ScopedAStatus authenticate(int32_t cookie, int64_t keystoreOperationId,
+ std::shared_ptr<common::ICancellationSignal>* out) override;
ndk::ScopedAStatus detectInteraction(
- int32_t cookie, std::shared_ptr<common::ICancellationSignal>* return_val) override;
+ int32_t cookie, std::shared_ptr<common::ICancellationSignal>* out) override;
ndk::ScopedAStatus enumerateEnrollments(int32_t cookie) override;
@@ -62,7 +61,7 @@
ndk::ScopedAStatus onUiReady() override;
private:
- std::shared_ptr<ISessionCallback> cb_;
+ std::shared_ptr<ISessionCallback> mCb;
};
} // namespace aidl::android::hardware::biometrics::fingerprint
diff --git a/biometrics/fingerprint/aidl/default/include/WorkerThread.h b/biometrics/fingerprint/aidl/default/include/WorkerThread.h
new file mode 100644
index 0000000..49104c8
--- /dev/null
+++ b/biometrics/fingerprint/aidl/default/include/WorkerThread.h
@@ -0,0 +1,74 @@
+/*
+ * 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
+
+#include <mutex>
+#include <optional>
+#include <queue>
+#include <thread>
+
+namespace aidl::android::hardware::biometrics::fingerprint {
+
+using Task = std::function<void()>;
+
+// 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.
+class WorkerThread final {
+ public:
+ // Internally creates a queue that cannot exceed maxQueueSize elements and a new thread that
+ // polls the queue for tasks until this instance is destructed.
+ explicit WorkerThread(size_t maxQueueSize);
+
+ // Unblocks the internal queue and calls join on the internal thread allowing it to gracefully
+ // exit.
+ ~WorkerThread();
+
+ // Disallow copying this class.
+ WorkerThread(const WorkerThread&) = delete;
+ WorkerThread& operator=(const WorkerThread&) = delete;
+
+ // Also disable moving this class to simplify implementation.
+ WorkerThread(WorkerThread&&) = delete;
+ WorkerThread& operator=(WorkerThread&&) = delete;
+
+ // 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);
+
+ private:
+ // The function that runs on the internal thread. Sequentially runs the available tasks from
+ // the queue. If the queue is empty, waits until a new task is added. If the worker is being
+ // destructed, finishes its current task and gracefully exits.
+ void threadFunc();
+
+ // The maximum size that the queue is allowed to expand to.
+ size_t mMaxSize;
+
+ // Whether the destructor was called. If true, tells threadFunc to exit as soon as possible, and
+ // tells schedule to avoid doing any work.
+ std::atomic<bool> mIsDestructing;
+
+ // Queue that's guarded by mQueueMutex and mQueueCond.
+ std::deque<Task> mQueue;
+ std::mutex mQueueMutex;
+ std::condition_variable mQueueCond;
+
+ // The internal thread that works on the tasks from the queue.
+ std::thread mThread;
+};
+
+} // namespace aidl::android::hardware::biometrics::fingerprint
diff --git a/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp b/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp
new file mode 100644
index 0000000..ba901ad
--- /dev/null
+++ b/biometrics/fingerprint/aidl/default/tests/WorkerThreadTest.cpp
@@ -0,0 +1,105 @@
+/*
+ * 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.
+ */
+
+#include <algorithm>
+#include <chrono>
+#include <future>
+#include <thread>
+
+#include <gtest/gtest.h>
+
+#include "WorkerThread.h"
+
+namespace {
+
+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([] {}));
+ // Allow enough time for the previous task to be processed.
+ std::this_thread::sleep_for(2ms);
+ }
+}
+
+TEST(WorkerThreadTest, ScheduleReturnsFalseWhenQueueIsFull) {
+ WorkerThread worker(2 /*maxQueueSize*/);
+ // Add a long-running task.
+ worker.schedule([] { 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([] {});
+
+ EXPECT_FALSE(worker.schedule([] {}));
+}
+
+TEST(WorkerThreadTest, TasksExecuteInOrder) {
+ constexpr int NUM_TASKS = 10000;
+ WorkerThread worker(NUM_TASKS);
+
+ std::vector<int> results;
+ for (int i = 0; i < NUM_TASKS; ++i) {
+ worker.schedule([&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(); });
+ auto status = future.wait_for(1s);
+ ASSERT_EQ(status, std::future_status::ready);
+
+ ASSERT_EQ(results.size(), NUM_TASKS);
+ EXPECT_TRUE(std::is_sorted(results.begin(), results.end()));
+}
+
+TEST(WorkerThreadTest, ExecutionStopsAfterWorkerIsDestroyed) {
+ std::promise<void> promise1;
+ std::promise<void> promise2;
+ auto future1 = promise1.get_future();
+ auto future2 = promise2.get_future();
+
+ {
+ WorkerThread worker(2 /*maxQueueSize*/);
+ worker.schedule([&promise1] {
+ promise1.set_value();
+ std::this_thread::sleep_for(200ms);
+ });
+ worker.schedule([&promise2] { promise2.set_value(); });
+
+ // Make sure the first task is executing.
+ auto status1 = future1.wait_for(1s);
+ ASSERT_EQ(status1, std::future_status::ready);
+ }
+
+ // The second task should never execute.
+ auto status2 = future2.wait_for(1s);
+ EXPECT_EQ(status2, std::future_status::timeout);
+}
+
+} // namespace