Properly initialize MotionClassifier
InputClassifier will now play a more active role in managing the lifecycle of MotionClassifier. In the previous version of the code, we had a circular reference: MotionClassifier owned a wp<InputClassifier>, that sometimes got promoted to sp<>. But the InputClassifier was the official owner of MotionClassifier, and owned a unique_ptr<MotionClassifier>. The owner of InputClassifier in real code is InputManager, and in test code, it's the test class. When the owner of InputClassifier destroyed InputClassifier, this could sometimes happen at the time when the MotionClassifier also held a reference to the InputClassifier. That meant that the proper owner of InputClassifier was not invoking the destructor. Instead, the MotionClassifier was invoking the InputClassifier destructor. To fix the situation, we now do the following: 1. InputClassifier will never die before MotionClassifier. 2. MotionClassifier constructor is now allowed to block. It would block for some time because calling getService takes a while. To account for this, InputClassifier launches a new thread to create MotionClassifier. 3. When MotionClassifier is ready to process events, InputClassifier updates mMotionClassifier, which makes it non-null. 4. We now create a separate death recipient, which is co-owned by InputClassifier and MotionClassifier. This is done so that the refcount of the deathrecipient does not affect the refcount of InputClassifier, and thus enforces the ownership of InputClassifier by the InputManager. Now, no one can call ~InputClassifier except for its real owner. 5. MotionClassifier will subscribe the death recipient to the death of the HAL. InputClassifier will delete MotionClassifier if HAL dies. MotionClassifier no longer holds on to the death recipient. 6. We move the loop of the MotionClassifier thread to focus only on processing events. That thread will no longer do any initialization. 7. Remove the thread check inside MotionClassifier. It isn't really useful, now that there's only 1 function for the entire thread. Ownership summary: Both InputClassifier and MotionClassifier own DeathRecipient. DeathRecipient has a reference to InputClassifier. Thus, we must guarantee that DeathRecipient dies before InputClassifier. InputClassifier owns MotionClassifier. This is OK, since even if InputClassifier dies, it will first delete MotionClassifier. That will cause MotionClassifier to release 1 refCount from DeathRecipient. That means the only reference remaining to DeathRecipient will be inside InputClassifier, so InputClassifier will always be alive until DeathRecipient is dead. Similar argument applies if MotionClassifier and DeathRecipient die in different order (as observed from InputClassifier). Tests: 1) Manually running inputflinger_tests on cuttlefish: build/launch cuttlefish using go/acloud m inputflinger_tests adb push out/target/product/vsoc_x86/data/nativetest/inputflinger_tests/inputflinger_tests /data/nativetest/inputflinger_tests/inputflinger_tests adb shell /data/nativetest/inputflinger_tests # ./inputflinger_tests --gtest_filter=*InputClassifierTest* --gtest_repeat=1000 --gtest_break_on_failure 2) Boot flame and open logcat. Observe in logcat: StartInputManagerService took to complete: 2ms Previously, in synchronous approach ( b/130184032) it was about 100 ms (so we did not regress). 3) Kill the HAL while system_server is running, and dumpsys input before and after: adb shell dumpsys input (observe that MotionClassifier is non-null on flame) adb shell -t killall android.hardware.input.classifier@1.0-service adb shell dumpsys input (observe that MotionCLassifier is null) Bug: 148311342 Test: see "Tests" section above Change-Id: Ic76b82bd5f2cd374e3b001400eb495ca36de7353 Merged-In: Ic76b82bd5f2cd374e3b001400eb495ca36de7353 (cherry picked from commit 1652397a)
Loading
Please register or sign in to comment