diff --git a/sensors/2.0/multihal/HalProxy.cpp b/sensors/2.0/multihal/HalProxy.cpp index d66721866c..fe2b98b21a 100644 --- a/sensors/2.0/multihal/HalProxy.cpp +++ b/sensors/2.0/multihal/HalProxy.cpp @@ -65,15 +65,7 @@ HalProxy::HalProxy(std::vector& subHalList) : mSubHalList(subHa } HalProxy::~HalProxy() { - mThreadsRun.store(false); - mWakelockCV.notify_one(); - mEventQueueWriteCV.notify_one(); - if (mPendingWritesThread.joinable()) { - mPendingWritesThread.join(); - } - if (mWakelockThread.joinable()) { - mWakelockThread.join(); - } + stopThreads(); } Return HalProxy::getSensorsList(getSensorsList_cb _hidl_cb) { @@ -119,8 +111,18 @@ Return HalProxy::initialize( const sp& sensorsCallback) { Result result = Result::OK; - // TODO: clean up sensor requests, if not already done elsewhere through a death recipient, and - // clean up any other resources that exist (FMQs, flags, threads, etc.) + stopThreads(); + resetSharedWakelock(); + + // So that the pending write events queue can be cleared safely and when we start threads + // again we do not get new events until after initialize resets the subhals. + disableAllSensors(); + + // Clears the queue if any events were pending write before. + mPendingWriteEventsQueue = std::queue, size_t>>(); + + // Clears previously connected dynamic sensors + mDynamicSensors.clear(); mDynamicSensorsCallback = sensorsCallback; @@ -128,21 +130,29 @@ Return HalProxy::initialize( mEventQueue = std::make_unique(eventQueueDescriptor, true /* resetPointers */); - // Create the EventFlag that is used to signal to the framework that sensor events have been - // written to the Event FMQ - if (EventFlag::createEventFlag(mEventQueue->getEventFlagWord(), &mEventQueueFlag) != OK) { - result = Result::BAD_VALUE; - } - // Create the Wake Lock FMQ that is used by the framework to communicate whenever WAKE_UP // events have been successfully read and handled by the framework. mWakeLockQueue = std::make_unique(wakeLockDescriptor, true /* resetPointers */); + if (mEventQueueFlag != nullptr) { + EventFlag::deleteEventFlag(&mEventQueueFlag); + } + if (mWakelockQueueFlag != nullptr) { + EventFlag::deleteEventFlag(&mWakelockQueueFlag); + } + if (EventFlag::createEventFlag(mEventQueue->getEventFlagWord(), &mEventQueueFlag) != OK) { + result = Result::BAD_VALUE; + } + if (EventFlag::createEventFlag(mWakeLockQueue->getEventFlagWord(), &mWakelockQueueFlag) != OK) { + result = Result::BAD_VALUE; + } if (!mDynamicSensorsCallback || !mEventQueue || !mWakeLockQueue || mEventQueueFlag == nullptr) { result = Result::BAD_VALUE; } + mThreadsRun.store(true); + mPendingWritesThread = std::thread(startPendingWritesThread, this); mWakelockThread = std::thread(startWakelockThread, this); @@ -157,6 +167,8 @@ Return HalProxy::initialize( } } + mCurrentOperationMode = OperationMode::NORMAL; + return result; } @@ -331,6 +343,41 @@ void HalProxy::init() { initializeSensorList(); } +void HalProxy::stopThreads() { + mThreadsRun.store(false); + if (mEventQueueFlag != nullptr && mEventQueue != nullptr) { + size_t numToRead = mEventQueue->availableToRead(); + std::vector events(numToRead); + mEventQueue->read(events.data(), numToRead); + mEventQueueFlag->wake(static_cast(EventQueueFlagBits::EVENTS_READ)); + } + if (mWakelockQueueFlag != nullptr && mWakeLockQueue != nullptr) { + uint32_t kZero = 0; + mWakeLockQueue->write(&kZero); + mWakelockQueueFlag->wake(static_cast(WakeLockQueueFlagBits::DATA_WRITTEN)); + } + mWakelockCV.notify_one(); + mEventQueueWriteCV.notify_one(); + if (mPendingWritesThread.joinable()) { + mPendingWritesThread.join(); + } + if (mWakelockThread.joinable()) { + mWakelockThread.join(); + } +} + +void HalProxy::disableAllSensors() { + for (const auto& sensorEntry : mSensors) { + int32_t sensorHandle = sensorEntry.first; + activate(sensorHandle, false /* enabled */); + } + std::lock_guard dynamicSensorsLock(mDynamicSensorsMutex); + for (const auto& sensorEntry : mDynamicSensors) { + int32_t sensorHandle = sensorEntry.first; + activate(sensorHandle, false /* enabled */); + } +} + void HalProxy::startPendingWritesThread(HalProxy* halProxy) { halProxy->handlePendingWrites(); } @@ -347,8 +394,6 @@ void HalProxy::handlePendingWrites() { size_t eventQueueSize = mEventQueue->getQuantumCount(); size_t numToWrite = std::min(pendingWriteEvents.size(), eventQueueSize); lock.unlock(); - // TODO: Find a way to interrup writeBlocking if the thread should exit - // so we don't have to wait for timeout on framework restarts. if (!mEventQueue->writeBlocking( pendingWriteEvents.data(), numToWrite, static_cast(EventQueueFlagBits::EVENTS_READ), diff --git a/sensors/2.0/multihal/include/HalProxy.h b/sensors/2.0/multihal/include/HalProxy.h index 6592afef96..26bc6447a0 100644 --- a/sensors/2.0/multihal/include/HalProxy.h +++ b/sensors/2.0/multihal/include/HalProxy.h @@ -149,9 +149,13 @@ class HalProxy : public ISensors, public IScopedWakelockRefCounter { std::unique_ptr mWakeLockQueue; /** - * Event Flag to signal to the framework when sensor events are available to be read + * Event Flag to signal to the framework when sensor events are available to be read and to + * interrupt event queue blocking write. */ - EventFlag* mEventQueueFlag; + EventFlag* mEventQueueFlag = nullptr; + + //! Event Flag to signal internally that the wakelock queue should stop its blocking read. + EventFlag* mWakelockQueueFlag = nullptr; /** * Callback to the sensors framework to inform it that new sensors have been added or removed. @@ -253,6 +257,16 @@ class HalProxy : public ISensors, public IScopedWakelockRefCounter { */ void init(); + /** + * Stops all threads by setting the threads running flag to false and joining to them. + */ + void stopThreads(); + + /** + * Disable all the sensors observed by the HalProxy. + */ + void disableAllSensors(); + /** * Starts the thread that handles pending writes to event fmq. * diff --git a/sensors/2.0/multihal/include/SubHal.h b/sensors/2.0/multihal/include/SubHal.h index e7eedaa3ea..92ae3a61df 100644 --- a/sensors/2.0/multihal/include/SubHal.h +++ b/sensors/2.0/multihal/include/SubHal.h @@ -130,11 +130,13 @@ class ISensorsSubHal : public ISensors { virtual const std::string getName() = 0; /** - * First method invoked on the sub-HAL after it's allocated through sensorsHalGetSubHal() by the - * HalProxy. Sub-HALs should use this to initialize any state and retain the callback given in - * order to communicate with the HalProxy. Method will be called anytime the sensors framework - * restarts. Therefore, this method will be responsible for reseting the state of the subhal and - * cleaning up and reallocating any previously allocated data. + * This is the first method invoked on the sub-HAL after it's allocated through + * sensorsHalGetSubHal() by the HalProxy. Sub-HALs should use this to initialize any state and + * retain the callback given in order to communicate with the HalProxy. Method will be called + * anytime the sensors framework restarts. Therefore, this method will be responsible for + * reseting the state of the subhal and cleaning up and reallocating any previously allocated + * data. Initialize should ensure that the subhal has reset its operation mode to NORMAL state + * as well. * * @param halProxyCallback callback used to inform the HalProxy when a dynamic sensor's state * changes, new sensor events should be sent to the framework, and when a new ScopedWakelock diff --git a/sensors/2.0/multihal/tests/HalProxy_test.cpp b/sensors/2.0/multihal/tests/HalProxy_test.cpp index fa527c9209..040e8c2ca4 100644 --- a/sensors/2.0/multihal/tests/HalProxy_test.cpp +++ b/sensors/2.0/multihal/tests/HalProxy_test.cpp @@ -432,7 +432,7 @@ TEST(HalProxyTest, PostEventsMultipleSubhalsThreaded) { EXPECT_EQ(eventQueue->availableToRead(), kNumEvents * 2); } -TEST(HalProxyTest, DestructingWithEventsPendingOnBackgroundThreadTest) { +TEST(HalProxyTest, DestructingWithEventsPendingOnBackgroundThread) { constexpr size_t kQueueSize = 5; constexpr size_t kNumEvents = 6; AllSensorsSubHal subHal; @@ -447,13 +447,145 @@ TEST(HalProxyTest, DestructingWithEventsPendingOnBackgroundThreadTest) { std::vector events = makeMultipleAccelerometerEvents(kNumEvents); subHal.postEvents(events, false /* wakeup */); - // Sleep for a half second so that background thread has time to attempt it's blocking write - std::this_thread::sleep_for(std::chrono::milliseconds(500)); + // Destructing HalProxy object with events on the background thread +} - // Should see a 5 second wait for blocking write timeout here +TEST(HalProxyTest, DestructingWithUnackedWakeupEventsPosted) { + constexpr size_t kQueueSize = 5; + AllSensorsSubHal subHal; + std::vector subHals{&subHal}; - // Should be one events left on pending writes queue here and proxy will destruct - // If this TEST completes then it was a success, if it hangs we will see a crash + std::unique_ptr eventQueue = makeEventFMQ(kQueueSize); + std::unique_ptr wakeLockQueue = makeWakelockFMQ(kQueueSize); + ::android::sp callback = new SensorsCallback(); + HalProxy proxy(subHals); + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback); + + std::vector events{makeProximityEvent()}; + subHal.postEvents(events, true /* wakeup */); + + // Not sending any acks back through wakeLockQueue + + // Destructing HalProxy object with unacked wakeup events posted +} + +TEST(HalProxyTest, ReinitializeWithEventsPendingOnBackgroundThread) { + constexpr size_t kQueueSize = 5; + constexpr size_t kNumEvents = 10; + AllSensorsSubHal subHal; + std::vector subHals{&subHal}; + + std::unique_ptr eventQueue = makeEventFMQ(kQueueSize); + std::unique_ptr wakeLockQueue = makeWakelockFMQ(kQueueSize); + ::android::sp callback = new SensorsCallback(); + HalProxy proxy(subHals); + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback); + + std::vector events = makeMultipleAccelerometerEvents(kNumEvents); + subHal.postEvents(events, false /* wakeup */); + + eventQueue = makeEventFMQ(kQueueSize); + wakeLockQueue = makeWakelockFMQ(kQueueSize); + + Result secondInitResult = + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback); + EXPECT_EQ(secondInitResult, Result::OK); + // Small sleep so that pending writes thread has a change to hit writeBlocking call. + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + Event eventOut; + EXPECT_FALSE(eventQueue->read(&eventOut)); +} + +TEST(HalProxyTest, ReinitializingWithUnackedWakeupEventsPosted) { + constexpr size_t kQueueSize = 5; + AllSensorsSubHal subHal; + std::vector subHals{&subHal}; + + std::unique_ptr eventQueue = makeEventFMQ(kQueueSize); + std::unique_ptr wakeLockQueue = makeWakelockFMQ(kQueueSize); + ::android::sp callback = new SensorsCallback(); + HalProxy proxy(subHals); + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback); + + std::vector events{makeProximityEvent()}; + subHal.postEvents(events, true /* wakeup */); + + // Not sending any acks back through wakeLockQueue + + eventQueue = makeEventFMQ(kQueueSize); + wakeLockQueue = makeWakelockFMQ(kQueueSize); + + Result secondInitResult = + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback); + EXPECT_EQ(secondInitResult, Result::OK); +} + +TEST(HalProxyTest, InitializeManyTimesInARow) { + constexpr size_t kQueueSize = 5; + constexpr size_t kNumTimesToInit = 100; + AllSensorsSubHal subHal; + std::vector subHals{&subHal}; + + std::unique_ptr eventQueue = makeEventFMQ(kQueueSize); + std::unique_ptr wakeLockQueue = makeWakelockFMQ(kQueueSize); + ::android::sp callback = new SensorsCallback(); + HalProxy proxy(subHals); + + for (size_t i = 0; i < kNumTimesToInit; i++) { + Result secondInitResult = + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback); + EXPECT_EQ(secondInitResult, Result::OK); + } +} + +TEST(HalProxyTest, OperationModeResetOnInitialize) { + constexpr size_t kQueueSize = 5; + AllSensorsSubHal subHal; + std::vector subHals{&subHal}; + std::unique_ptr eventQueue = makeEventFMQ(kQueueSize); + std::unique_ptr wakeLockQueue = makeWakelockFMQ(kQueueSize); + ::android::sp callback = new SensorsCallback(); + HalProxy proxy(subHals); + proxy.setOperationMode(OperationMode::DATA_INJECTION); + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callback); + Event event = makeAccelerometerEvent(); + // Should not be able to inject a non AdditionInfo type event because operation mode should + // have been reset to NORMAL + EXPECT_EQ(proxy.injectSensorData(event), Result::BAD_VALUE); +} + +TEST(HalProxyTest, DynamicSensorsDiscardedOnInitialize) { + constexpr size_t kQueueSize = 5; + constexpr size_t kNumSensors = 5; + AddAndRemoveDynamicSensorsSubHal subHal; + std::vector subHals{&subHal}; + std::unique_ptr eventQueue = makeEventFMQ(kQueueSize); + std::unique_ptr wakeLockQueue = makeWakelockFMQ(kQueueSize); + HalProxy proxy(subHals); + + std::vector sensorsToConnect; + std::vector sensorHandlesToAttemptToRemove; + makeSensorsAndSensorHandlesStartingAndOfSize(1, kNumSensors, sensorsToConnect, + sensorHandlesToAttemptToRemove); + + std::vector nonDynamicSensorHandles; + for (int32_t sensorHandle = 1; sensorHandle < 10; sensorHandle++) { + nonDynamicSensorHandles.push_back(sensorHandle); + } + + TestSensorsCallback* callback = new TestSensorsCallback(); + ::android::sp callbackPtr = callback; + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callbackPtr); + subHal.addDynamicSensors(sensorsToConnect); + + proxy.initialize(*eventQueue->getDesc(), *wakeLockQueue->getDesc(), callbackPtr); + subHal.removeDynamicSensors(sensorHandlesToAttemptToRemove); + + std::vector sensorHandlesActuallyRemoved = callback->getSensorHandlesDisconnected(); + + // Should not have received the sensorHandles for any dynamic sensors that were removed since + // all of them should have been removed in the second initialize call. + EXPECT_TRUE(sensorHandlesActuallyRemoved.empty()); } TEST(HalProxyTest, DynamicSensorsConnectedTest) { diff --git a/sensors/2.0/multihal/tests/fake_subhal/SensorsSubHal.cpp b/sensors/2.0/multihal/tests/fake_subhal/SensorsSubHal.cpp index cf3ae7560d..0da424622f 100644 --- a/sensors/2.0/multihal/tests/fake_subhal/SensorsSubHal.cpp +++ b/sensors/2.0/multihal/tests/fake_subhal/SensorsSubHal.cpp @@ -158,6 +158,7 @@ Return SensorsSubHal::debug(const hidl_handle& fd, const hidl_vec SensorsSubHal::initialize(const sp& halProxyCallback) { mCallback = halProxyCallback; + setOperationMode(OperationMode::NORMAL); return Result::OK; }