diff options
author | Andrew Scull <ascull@google.com> | 2018-04-09 12:19:33 +0100 |
---|---|---|
committer | Andrew Scull <ascull@google.com> | 2018-04-09 12:26:44 +0100 |
commit | ec24f14bad5a291cb8ffde7012ab68336ed3b05e (patch) | |
tree | d9ecd7bdf80f3089db3a6efb7e982736c02d0863 | |
parent | 688b42460f9dcf9807729d70467c27865629424d (diff) | |
download | android-ec24f14bad5a291cb8ffde7012ab68336ed3b05e.tar.gz |
Citadeld: handle requests on multiple threads
Ensure that each app only has one client at a time but allow different
apps to be accessed concurrently. The driver is thread safe so there is
no need to synchronize driver access.
This also allows a hard reset request to be processed if there is a
blocking transaction going on.
Bug: 77728719
Change-Id: I75d5ce8d23d7562d7b6e718ff4925ffad0b637a3
-rw-r--r-- | citadel/citadeld/main.cpp | 31 |
1 files changed, 23 insertions, 8 deletions
diff --git a/citadel/citadeld/main.cpp b/citadel/citadeld/main.cpp index 3fc9243..2afbdb6 100644 --- a/citadel/citadeld/main.cpp +++ b/citadel/citadeld/main.cpp @@ -16,6 +16,7 @@ #include <limits> #include <thread> +#include <mutex> #include <android-base/logging.h> #include <binder/IPCThreadState.h> @@ -43,32 +44,46 @@ using ::android::hardware::citadel::ICitadeld; namespace { -struct CitadelProxy : public BnCitadeld { - NuggetClient& _client; - +class CitadelProxy : public BnCitadeld { +public: CitadelProxy(NuggetClient& client) : _client{client} {} ~CitadelProxy() override = default; Status callApp(const int32_t _appId, const int32_t _arg, const std::vector<uint8_t>& request, std::vector<uint8_t>* const response, int32_t* const _aidl_return) override { - // AIDL doesn't support 16-bit integers so validate it before casting - if (_arg > std::numeric_limits<uint16_t>::max()) { + // AIDL doesn't support integers less than 32-bit so validate it before casting + if (_appId < 0 || _appId > kMaxAppId) { + LOG(ERROR) << "App ID " << _appId << " is outside the app ID range"; + return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT); + } + if (_arg < 0 || _arg > std::numeric_limits<uint16_t>::max()) { + LOG(ERROR) << "Argument " << _arg << " is outside the unsigned 16-bit range"; return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT); } - const uint32_t appId = static_cast<uint32_t>(_appId); + const uint8_t appId = static_cast<uint32_t>(_appId); const uint16_t arg = static_cast<uint16_t>(_arg); uint32_t* const appStatus = reinterpret_cast<uint32_t*>(_aidl_return); + // Make the call to the app while holding the lock for that app + std::unique_lock<std::mutex> lock(_appLocks[appId]); *appStatus = _client.CallApp(appId, arg, request, response); return Status::ok(); } Status reset(bool* const _aidl_return) override { + // This doesn't use the transport API to talk to any app so doesn't need + // to hold any app locks. const nos_device& device = *_client.Device(); *_aidl_return = (device.ops.reset(device.ctx) == 0); return Status::ok(); } + +private: + static constexpr auto kMaxAppId = std::numeric_limits<uint8_t>::max(); + + NuggetClient& _client; + std::mutex _appLocks[kMaxAppId + 1]; }; [[noreturn]] void CitadelEventDispatcher(const nos_device& device) { @@ -113,8 +128,8 @@ int main() { // registered listeners. std::thread event_dispatcher(CitadelEventDispatcher, *citadel.Device()); - // The driver only support single threaded access so we only need to process - // Binder transactions on a single thread. + // Start handling binder requests with multiple threads + ProcessState::self()->startThreadPool(); IPCThreadState::self()->joinThreadPool(); return 0; } |