From 9ebf233e8d1c6a6be6e75c71c5be0a3531008b94 Mon Sep 17 00:00:00 2001 From: Luke Huang Date: Sat, 20 Jun 2020 13:01:33 +0800 Subject: Backport: Update the documentation of ANDROID_RESOLV_NO_CACHE_STORE Make it up-to-date. Test: N/A Bug: 150371903 Change-Id: Ia1402a18d6d466ffbb0357127d7d45cf6c722550 (cherry picked from commit f1cf6a632da354cff3d8aed54913e1ee2909908e) --- include/android/multinetwork.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/android/multinetwork.h b/include/android/multinetwork.h index d31d1f122f..f7294a9ea5 100644 --- a/include/android/multinetwork.h +++ b/include/android/multinetwork.h @@ -123,8 +123,8 @@ enum ResNsendFlags : uint32_t { ANDROID_RESOLV_NO_RETRY = 1 << 0, /** - * Do not cache the result of the lookup. The lookup may return a result that is already - * in the cache, unless the ANDROID_RESOLV_NO_CACHE_LOOKUP flag is also specified. + * Don't lookup this request in the cache, and don't cache the result of the lookup. + * This flag implies {@link #ANDROID_RESOLV_NO_CACHE_LOOKUP}. */ ANDROID_RESOLV_NO_CACHE_STORE = 1 << 1, -- cgit v1.2.3-59-g8ed1b From 09052cc6655ab2756c4c484b958cdaf29325e652 Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Tue, 22 Sep 2020 13:05:15 -0700 Subject: Prevent mEventCache UAF in SensorEventConnection Since there is no check to see if SensorEventConnection has been destroyed, the mEventCache pointer can still be used even after it was freed. Bug: 168211968 Test: Run test code that attempts to enable a sensor after destroying the SensorEventConnection, and verify no system_server crash occurs. Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 --- services/sensorservice/SensorEventConnection.cpp | 28 +++++++++++++++--------- services/sensorservice/SensorEventConnection.h | 5 +++-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index 0a05dd1b18..15ca7ac3cb 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -44,20 +45,13 @@ SensorService::SensorEventConnection::SensorEventConnection( SensorService::SensorEventConnection::~SensorEventConnection() { ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this); destroy(); -} - -void SensorService::SensorEventConnection::destroy() { - Mutex::Autolock _l(mDestroyLock); - - // destroy once only - if (mDestroyed) { - return; - } - mService->cleanupConnection(this); if (mEventCache != NULL) { delete mEventCache; } +} + +void SensorService::SensorEventConnection::destroy() { mDestroyed = true; } @@ -525,6 +519,11 @@ status_t SensorService::SensorEventConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + status_t err; if (enabled) { err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs, @@ -539,10 +538,19 @@ status_t SensorService::SensorEventConnection::enableDisable( status_t SensorService::SensorEventConnection::setEventRate( int handle, nsecs_t samplingPeriodNs) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName); } status_t SensorService::SensorEventConnection::flush() { + if (mDestroyed) { + return DEAD_OBJECT; + } + return mService->flushSensor(this, mOpPackageName); } diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 6f282cdc60..bd1b3bbef7 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include #include #include @@ -165,8 +166,8 @@ private: int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; }; } // namepsace android -- cgit v1.2.3-59-g8ed1b From 3b27b3b3bb36274171a9abb31f3e3f6b53036cf0 Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Tue, 22 Sep 2020 13:05:15 -0700 Subject: Prevent mEventCache UAF in SensorEventConnection Since there is no check to see if SensorEventConnection has been destroyed, the mEventCache pointer can still be used even after it was freed. Bug: 168211968 Test: Run test code that attempts to enable a sensor after destroying the SensorEventConnection, and verify no system_server crash occurs. Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 --- services/sensorservice/SensorEventConnection.cpp | 28 +++++++++++++++--------- services/sensorservice/SensorEventConnection.h | 6 +++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index 956844f426..a9e8321dcf 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -44,20 +45,13 @@ SensorService::SensorEventConnection::SensorEventConnection( SensorService::SensorEventConnection::~SensorEventConnection() { ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this); destroy(); -} - -void SensorService::SensorEventConnection::destroy() { - Mutex::Autolock _l(mDestroyLock); - - // destroy once only - if (mDestroyed) { - return; - } - mService->cleanupConnection(this); if (mEventCache != NULL) { delete mEventCache; } +} + +void SensorService::SensorEventConnection::destroy() { mDestroyed = true; } @@ -555,6 +549,11 @@ status_t SensorService::SensorEventConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + status_t err; if (enabled) { err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs, @@ -569,10 +568,19 @@ status_t SensorService::SensorEventConnection::enableDisable( status_t SensorService::SensorEventConnection::setEventRate( int handle, nsecs_t samplingPeriodNs) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName); } status_t SensorService::SensorEventConnection::flush() { + if (mDestroyed) { + return DEAD_OBJECT; + } + return mService->flushSensor(this, mOpPackageName); } diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 032721ea39..84b4d86879 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include #include #include @@ -168,8 +169,9 @@ private: int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; + bool mHasSensorAccess; }; -- cgit v1.2.3-59-g8ed1b From bbf3b377482aeac64c23556bcb2a129d944629ed Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Tue, 22 Sep 2020 13:05:15 -0700 Subject: Prevent mEventCache UAF in SensorEventConnection Since there is no check to see if SensorEventConnection has been destroyed, the mEventCache pointer can still be used even after it was freed. Bug: 168211968 Test: Run test code that attempts to enable a sensor after destroying the SensorEventConnection, and verify no system_server crash occurs. Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 --- services/sensorservice/SensorEventConnection.cpp | 28 +++++++++++++++--------- services/sensorservice/SensorEventConnection.h | 6 +++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index 0e409409f2..1811716af9 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -45,20 +46,13 @@ SensorService::SensorEventConnection::SensorEventConnection( SensorService::SensorEventConnection::~SensorEventConnection() { ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this); destroy(); -} - -void SensorService::SensorEventConnection::destroy() { - Mutex::Autolock _l(mDestroyLock); - - // destroy once only - if (mDestroyed) { - return; - } - mService->cleanupConnection(this); if (mEventCache != nullptr) { delete[] mEventCache; } +} + +void SensorService::SensorEventConnection::destroy() { mDestroyed = true; } @@ -603,6 +597,11 @@ status_t SensorService::SensorEventConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + status_t err; if (enabled) { err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs, @@ -617,10 +616,19 @@ status_t SensorService::SensorEventConnection::enableDisable( status_t SensorService::SensorEventConnection::setEventRate( int handle, nsecs_t samplingPeriodNs) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName); } status_t SensorService::SensorEventConnection::flush() { + if (mDestroyed) { + return DEAD_OBJECT; + } + return mService->flushSensor(this, mOpPackageName); } diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index fd881cbc0b..d3528551bb 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include #include #include #include @@ -182,8 +183,9 @@ private: int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; + bool mHasSensorAccess; // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a -- cgit v1.2.3-59-g8ed1b From 9fb3ea9c93b98a77e7899299019542e333da21fb Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Tue, 22 Sep 2020 13:05:15 -0700 Subject: Prevent mEventCache UAF in SensorEventConnection Since there is no check to see if SensorEventConnection has been destroyed, the mEventCache pointer can still be used even after it was freed. Bug: 168211968 Test: Run test code that attempts to enable a sensor after destroying the SensorEventConnection, and verify no system_server crash occurs. Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 --- services/sensorservice/SensorEventConnection.cpp | 28 +++++++++++++++--------- services/sensorservice/SensorEventConnection.h | 6 +++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index 0e409409f2..1811716af9 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -45,20 +46,13 @@ SensorService::SensorEventConnection::SensorEventConnection( SensorService::SensorEventConnection::~SensorEventConnection() { ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this); destroy(); -} - -void SensorService::SensorEventConnection::destroy() { - Mutex::Autolock _l(mDestroyLock); - - // destroy once only - if (mDestroyed) { - return; - } - mService->cleanupConnection(this); if (mEventCache != nullptr) { delete[] mEventCache; } +} + +void SensorService::SensorEventConnection::destroy() { mDestroyed = true; } @@ -603,6 +597,11 @@ status_t SensorService::SensorEventConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + status_t err; if (enabled) { err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs, @@ -617,10 +616,19 @@ status_t SensorService::SensorEventConnection::enableDisable( status_t SensorService::SensorEventConnection::setEventRate( int handle, nsecs_t samplingPeriodNs) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName); } status_t SensorService::SensorEventConnection::flush() { + if (mDestroyed) { + return DEAD_OBJECT; + } + return mService->flushSensor(this, mOpPackageName); } diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index fd881cbc0b..d3528551bb 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include #include #include #include @@ -182,8 +183,9 @@ private: int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; + bool mHasSensorAccess; // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a -- cgit v1.2.3-59-g8ed1b From f1bf7dd095ac2f632442663cb16aeef4691b93e7 Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Tue, 22 Sep 2020 13:05:15 -0700 Subject: Prevent mEventCache UAF in SensorEventConnection Since there is no check to see if SensorEventConnection has been destroyed, the mEventCache pointer can still be used even after it was freed. Bug: 168211968 Test: Run test code that attempts to enable a sensor after destroying the SensorEventConnection, and verify no system_server crash occurs. Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 (cherry picked from commit 3e9afc163256db661b9039120d07501b3a8a7d99) --- services/sensorservice/SensorEventConnection.cpp | 28 +++++++++++++++--------- services/sensorservice/SensorEventConnection.h | 5 +++-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index b4b5f98609..6c8671289d 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -47,20 +48,13 @@ SensorService::SensorEventConnection::SensorEventConnection( SensorService::SensorEventConnection::~SensorEventConnection() { ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this); destroy(); -} - -void SensorService::SensorEventConnection::destroy() { - Mutex::Autolock _l(mDestroyLock); - - // destroy once only - if (mDestroyed) { - return; - } - mService->cleanupConnection(this); if (mEventCache != nullptr) { delete[] mEventCache; } +} + +void SensorService::SensorEventConnection::destroy() { mDestroyed = true; } @@ -665,6 +659,11 @@ status_t SensorService::SensorEventConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + status_t err; if (enabled) { err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs, @@ -679,10 +678,19 @@ status_t SensorService::SensorEventConnection::enableDisable( status_t SensorService::SensorEventConnection::setEventRate( int handle, nsecs_t samplingPeriodNs) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName); } status_t SensorService::SensorEventConnection::flush() { + if (mDestroyed) { + return DEAD_OBJECT; + } + return mService->flushSensor(this, mOpPackageName); } diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 8f2d5db28f..9487a39a92 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include #include #include #include @@ -182,8 +183,8 @@ private: int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a // valid mapping for sensors that require a permission in order to reduce the lookup time. -- cgit v1.2.3-59-g8ed1b From 930b583557e46f14b6d0a0dc1700eb33f18a7714 Mon Sep 17 00:00:00 2001 From: Arthur Ishiguro Date: Tue, 22 Sep 2020 13:05:15 -0700 Subject: Prevent mEventCache UAF in SensorEventConnection Since there is no check to see if SensorEventConnection has been destroyed, the mEventCache pointer can still be used even after it was freed. Bug: 168211968 Test: Run test code that attempts to enable a sensor after destroying the SensorEventConnection, and verify no system_server crash occurs. Change-Id: Ia9275b7cc574df371cdb2e1b80c6699df193b580 Merged-In: Ia9275b7cc574df371cdb2e1b80c6699df193b580 (cherry picked from commit 3e9afc163256db661b9039120d07501b3a8a7d99) --- services/sensorservice/SensorEventConnection.cpp | 28 +++++++++++++++--------- services/sensorservice/SensorEventConnection.h | 5 +++-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/services/sensorservice/SensorEventConnection.cpp b/services/sensorservice/SensorEventConnection.cpp index d14a3014c8..3cccaf9329 100644 --- a/services/sensorservice/SensorEventConnection.cpp +++ b/services/sensorservice/SensorEventConnection.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -53,20 +54,13 @@ SensorService::SensorEventConnection::SensorEventConnection( SensorService::SensorEventConnection::~SensorEventConnection() { ALOGD_IF(DEBUG_CONNECTIONS, "~SensorEventConnection(%p)", this); destroy(); -} - -void SensorService::SensorEventConnection::destroy() { - Mutex::Autolock _l(mDestroyLock); - - // destroy once only - if (mDestroyed) { - return; - } - mService->cleanupConnection(this); if (mEventCache != nullptr) { delete[] mEventCache; } +} + +void SensorService::SensorEventConnection::destroy() { mDestroyed = true; } @@ -679,6 +673,11 @@ status_t SensorService::SensorEventConnection::enableDisable( int handle, bool enabled, nsecs_t samplingPeriodNs, nsecs_t maxBatchReportLatencyNs, int reservedFlags) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + status_t err; if (enabled) { err = mService->enable(this, handle, samplingPeriodNs, maxBatchReportLatencyNs, @@ -693,10 +692,19 @@ status_t SensorService::SensorEventConnection::enableDisable( status_t SensorService::SensorEventConnection::setEventRate( int handle, nsecs_t samplingPeriodNs) { + if (mDestroyed) { + android_errorWriteLog(0x534e4554, "168211968"); + return DEAD_OBJECT; + } + return mService->setEventRate(this, handle, samplingPeriodNs, mOpPackageName); } status_t SensorService::SensorEventConnection::flush() { + if (mDestroyed) { + return DEAD_OBJECT; + } + return mService->flushSensor(this, mOpPackageName); } diff --git a/services/sensorservice/SensorEventConnection.h b/services/sensorservice/SensorEventConnection.h index 8f2d5db28f..9487a39a92 100644 --- a/services/sensorservice/SensorEventConnection.h +++ b/services/sensorservice/SensorEventConnection.h @@ -17,6 +17,7 @@ #ifndef ANDROID_SENSOR_EVENT_CONNECTION_H #define ANDROID_SENSOR_EVENT_CONNECTION_H +#include #include #include #include @@ -182,8 +183,8 @@ private: int mTotalAcksNeeded, mTotalAcksReceived; #endif - mutable Mutex mDestroyLock; - bool mDestroyed; + // Used to track if this object was inappropriately used after destroy(). + std::atomic_bool mDestroyed; // Store a mapping of sensor handles to required AppOp for a sensor. This map only contains a // valid mapping for sensors that require a permission in order to reduce the lookup time. -- cgit v1.2.3-59-g8ed1b From 181353f5c41609b3c969f10595a2379da427183f Mon Sep 17 00:00:00 2001 From: Chris Ye Date: Tue, 22 Sep 2020 15:36:28 -0700 Subject: Fix InputDevice listener notification for merged controller. Merged game controllers report unique device id combined from multiple input devices, when notifying device listener input manager will send a list of devices unique to each other. When an event hub device is added to a input device, the generation Id must be bumped to generate a device changed event for input device listener. Fixed the Sony game controller connection failure. Fixed the duplicate device listing in input service dump. Bug: 168338098 Test: atest inputflinger_tests Change-Id: Ie4f57215880ff06264a7c4f80c505cf4acc6bca5 --- services/inputflinger/reader/InputDevice.cpp | 6 +++- services/inputflinger/reader/InputReader.cpp | 36 ++++++++++++++++++---- services/inputflinger/reader/include/InputDevice.h | 2 +- services/inputflinger/reader/include/InputReader.h | 5 +++ 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp index 4b19e5e353..3347ba6ad7 100644 --- a/services/inputflinger/reader/InputDevice.cpp +++ b/services/inputflinger/reader/InputDevice.cpp @@ -84,12 +84,13 @@ void InputDevice::setEnabled(bool enabled, nsecs_t when) { bumpGeneration(); } -void InputDevice::dump(std::string& dump) { +void InputDevice::dump(std::string& dump, const std::string& eventHubDevStr) { InputDeviceInfo deviceInfo; getDeviceInfo(&deviceInfo); dump += StringPrintf(INDENT "Device %d: %s\n", deviceInfo.getId(), deviceInfo.getDisplayName().c_str()); + dump += StringPrintf(INDENT "%s", eventHubDevStr.c_str()); dump += StringPrintf(INDENT2 "Generation: %d\n", mGeneration); dump += StringPrintf(INDENT2 "IsExternal: %s\n", toString(mIsExternal)); dump += StringPrintf(INDENT2 "AssociatedDisplayPort: "); @@ -101,6 +102,7 @@ void InputDevice::dump(std::string& dump) { dump += StringPrintf(INDENT2 "HasMic: %s\n", toString(mHasMic)); dump += StringPrintf(INDENT2 "Sources: 0x%08x\n", deviceInfo.getSources()); dump += StringPrintf(INDENT2 "KeyboardType: %d\n", deviceInfo.getKeyboardType()); + dump += StringPrintf(INDENT2 "ControllerNum: %d\n", deviceInfo.getControllerNumber()); const std::vector& ranges = deviceInfo.getMotionRanges(); if (!ranges.empty()) { @@ -200,6 +202,8 @@ void InputDevice::addEventHubDevice(int32_t eventHubId, bool populateMappers) { // insert the context into the devices set mDevices.insert({eventHubId, std::make_pair(std::move(contextPtr), std::move(mappers))}); + // Must change generation to flag this device as changed + bumpGeneration(); } void InputDevice::removeEventHubDevice(int32_t eventHubId) { diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 657a134865..fc063f97ab 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -206,6 +206,14 @@ void InputReader::addDeviceLocked(nsecs_t when, int32_t eventHubId) { } mDevices.emplace(eventHubId, device); + // Add device to device to EventHub ids map. + const auto mapIt = mDeviceToEventHubIdsMap.find(device); + if (mapIt == mDeviceToEventHubIdsMap.end()) { + std::vector ids = {eventHubId}; + mDeviceToEventHubIdsMap.emplace(device, ids); + } else { + mapIt->second.push_back(eventHubId); + } bumpGenerationLocked(); if (device->getClasses() & INPUT_DEVICE_CLASS_EXTERNAL_STYLUS) { @@ -222,6 +230,17 @@ void InputReader::removeDeviceLocked(nsecs_t when, int32_t eventHubId) { std::shared_ptr device = std::move(deviceIt->second); mDevices.erase(deviceIt); + // Erase device from device to EventHub ids map. + auto mapIt = mDeviceToEventHubIdsMap.find(device); + if (mapIt != mDeviceToEventHubIdsMap.end()) { + std::vector& eventHubIds = mapIt->second; + eventHubIds.erase(std::remove_if(eventHubIds.begin(), eventHubIds.end(), + [eventHubId](int32_t eId) { return eId == eventHubId; }), + eventHubIds.end()); + if (eventHubIds.size() == 0) { + mDeviceToEventHubIdsMap.erase(mapIt); + } + } bumpGenerationLocked(); if (device->isIgnored()) { @@ -449,8 +468,7 @@ void InputReader::getInputDevices(std::vector& outInputDevices) void InputReader::getInputDevicesLocked(std::vector& outInputDevices) { outInputDevices.clear(); - for (auto& devicePair : mDevices) { - std::shared_ptr& device = devicePair.second; + for (const auto& [device, eventHubIds] : mDeviceToEventHubIdsMap) { if (!device->isIgnored()) { InputDeviceInfo info; device->getDeviceInfo(&info); @@ -621,11 +639,17 @@ void InputReader::dump(std::string& dump) { mEventHub->dump(dump); dump += "\n"; - dump += "Input Reader State:\n"; + dump += StringPrintf("Input Reader State (Nums of device: %zu):\n", + mDeviceToEventHubIdsMap.size()); - for (const auto& devicePair : mDevices) { - const std::shared_ptr& device = devicePair.second; - device->dump(dump); + for (const auto& devicePair : mDeviceToEventHubIdsMap) { + const std::shared_ptr& device = devicePair.first; + std::string eventHubDevStr = INDENT "EventHub Devices: [ "; + for (const auto& eId : devicePair.second) { + eventHubDevStr += StringPrintf("%d ", eId); + } + eventHubDevStr += "] \n"; + device->dump(dump, eventHubDevStr); } dump += INDENT "Configuration:\n"; diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h index 71313fc86f..a08062adef 100644 --- a/services/inputflinger/reader/include/InputDevice.h +++ b/services/inputflinger/reader/include/InputDevice.h @@ -66,7 +66,7 @@ public: bool isEnabled(); void setEnabled(bool enabled, nsecs_t when); - void dump(std::string& dump); + void dump(std::string& dump, const std::string& eventHubDevStr); void addEventHubDevice(int32_t eventHubId, bool populateMappers = true); void removeEventHubDevice(int32_t eventHubId); void configure(nsecs_t when, const InputReaderConfiguration* config, uint32_t changes); diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h index 693ec30b7d..d46ec1a783 100644 --- a/services/inputflinger/reader/include/InputReader.h +++ b/services/inputflinger/reader/include/InputReader.h @@ -141,6 +141,11 @@ private: // to lookup the input device instance from the EventHub device id. std::unordered_map> mDevices; + // An input device contains one or more eventHubId, this map provides a way to lookup the + // EventHubIds contained in the input device from the input device instance. + std::unordered_map, std::vector /*eventHubId*/> + mDeviceToEventHubIdsMap; + // low-level input event decoding and device management void processEventsLocked(const RawEvent* rawEvents, size_t count); -- cgit v1.2.3-59-g8ed1b From 162f643dded48b230888139d1e4c8a5e2013a7cd Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Thu, 8 Oct 2020 18:40:47 +0000 Subject: SurfaceFlinger: check for nullptr on setFrameRate Make sure that the layer still exists before trying to set the frame rate for it. Bug: 170027345 Change-Id: Ice1edd7c5320f4d1d915643e30430919de258dbc Test: manual --- services/surfaceflinger/SurfaceFlinger.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 148749e6f0..c9e595abd1 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6227,6 +6227,11 @@ status_t SurfaceFlinger::setFrameRate(const sp& surface, Mutex::Autolock lock(mStateLock); if (authenticateSurfaceTextureLocked(surface)) { sp layer = (static_cast(surface.get()))->getLayer(); + if (layer == nullptr) { + ALOGE("Attempt to set frame rate on a layer that no longer exists"); + return BAD_VALUE; + } + if (layer->setFrameRate( Layer::FrameRate(frameRate, Layer::FrameRate::convertCompatibility(compatibility)))) { -- cgit v1.2.3-59-g8ed1b From b432a59d19cd881d892c87d5d48254a71b106e22 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 16 Oct 2020 23:46:04 +0000 Subject: Update VNDK version for common graphics HALs. We are adding things to them, and in order for core libraries like libui to use them, the newer versions need to be in the VNDK. Exempt-From-Owner-Approval: cherry-pick Bug: 170435409 Test: build Change-Id: I84fc02127254c6b9d6d7858e140bd256b0dff44b Merged-In: I84fc02127254c6b9d6d7858e140bd256b0dff44b --- libs/gralloc/types/Android.bp | 4 ++-- libs/ui/Android.bp | 4 ++-- services/surfaceflinger/tests/Android.bp | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libs/gralloc/types/Android.bp b/libs/gralloc/types/Android.bp index 66fb295a19..cc31cd5f24 100644 --- a/libs/gralloc/types/Android.bp +++ b/libs/gralloc/types/Android.bp @@ -38,14 +38,14 @@ cc_library { ], shared_libs: [ - "android.hardware.graphics.common-ndk_platform", + "android.hardware.graphics.common-unstable-ndk_platform", "android.hardware.graphics.mapper@4.0", "libhidlbase", "liblog", ], export_shared_lib_headers: [ - "android.hardware.graphics.common-ndk_platform", + "android.hardware.graphics.common-unstable-ndk_platform", "android.hardware.graphics.mapper@4.0", "libhidlbase", ], diff --git a/libs/ui/Android.bp b/libs/ui/Android.bp index 1ee8c7105c..47eb59fb0a 100644 --- a/libs/ui/Android.bp +++ b/libs/ui/Android.bp @@ -72,7 +72,7 @@ cc_library_shared { "android.hardware.graphics.allocator@2.0", "android.hardware.graphics.allocator@3.0", "android.hardware.graphics.allocator@4.0", - "android.hardware.graphics.common-ndk_platform", + "android.hardware.graphics.common-unstable-ndk_platform", "android.hardware.graphics.common@1.2", "android.hardware.graphics.mapper@2.0", "android.hardware.graphics.mapper@2.1", @@ -89,7 +89,7 @@ cc_library_shared { export_shared_lib_headers: [ "android.hardware.graphics.common@1.2", - "android.hardware.graphics.common-ndk_platform", + "android.hardware.graphics.common-unstable-ndk_platform", "android.hardware.graphics.mapper@4.0", "libgralloctypes", ], diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp index fe2af80c98..1532855089 100644 --- a/services/surfaceflinger/tests/Android.bp +++ b/services/surfaceflinger/tests/Android.bp @@ -44,7 +44,7 @@ cc_test { "libtrace_proto", ], shared_libs: [ - "android.hardware.graphics.common-ndk_platform", + "android.hardware.graphics.common-unstable-ndk_platform", "android.hardware.graphics.common@1.2", "android.hardware.graphics.composer@2.1", "libandroid", -- cgit v1.2.3-59-g8ed1b From a87b5f88b56418a4d3fc0896ba62379e3c761a29 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 6 Oct 2020 13:42:41 +0800 Subject: Fixes De data is not backed up correctly when user locked (1/n) https://android.googlesource.com/platform/frameworks/native/+/fda19ecdd6df8f43d3368781a633792723fb9965/cmds/installd/InstalldNativeService.cpp#933 When an APK is stage installed, snapshotAppData(..., FLAG_STORAGE_DE) is called before user unlocked. The function bails out early at #937 because the folder is still encrypted. We should move the if block at #958 above so we can back up De data correctly even when Ce directories not ready. Note we have to check if the De folder exists before making a copy. (Cherry-picked from 1d643c77d3fcb7dfe0bf4dabed2964bbfca04c6f) Bug: 169594054 Test: atest StagedRollbackTest Merged-In: I2ca810bd9495de3bed58378a41b47863c6e8f8dd Change-Id: I2ca810bd9495de3bed58378a41b47863c6e8f8dd --- cmds/installd/InstalldNativeService.cpp | 51 +++++++++++++++++---------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index b9c1addf89..01eb3fe44f 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -940,6 +940,33 @@ binder::Status InstalldNativeService::snapshotAppData( auto scope_guard = android::base::make_scope_guard(deleter); + if (storageFlags & FLAG_STORAGE_DE) { + auto from = create_data_user_de_package_path(volume_uuid, user, package_name); + auto to = create_data_misc_de_rollback_path(volume_uuid, user, snapshotId); + auto rollback_package_path = create_data_misc_de_rollback_package_path(volume_uuid, user, + snapshotId, package_name); + + int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode); + if (rc != 0) { + return error(rc, "Failed to create folder " + to); + } + + rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */); + if (rc != 0) { + return error(rc, "Failed clearing existing snapshot " + rollback_package_path); + } + + // Check if we have data to copy. + if (access(from.c_str(), F_OK) == 0) { + rc = copy_directory_recursive(from.c_str(), to.c_str()); + } + if (rc != 0) { + res = error(rc, "Failed copying " + from + " to " + to); + clear_de_on_exit = true; + return res; + } + } + // The app may not have any data at all, in which case it's OK to skip here. auto from_ce = create_data_user_ce_package_path(volume_uuid, user, package_name); if (access(from_ce.c_str(), F_OK) != 0) { @@ -965,30 +992,6 @@ binder::Status InstalldNativeService::snapshotAppData( LOG(WARNING) << "Failed to clear code_cache of app " << packageName; } - if (storageFlags & FLAG_STORAGE_DE) { - auto from = create_data_user_de_package_path(volume_uuid, user, package_name); - auto to = create_data_misc_de_rollback_path(volume_uuid, user, snapshotId); - auto rollback_package_path = create_data_misc_de_rollback_package_path(volume_uuid, user, - snapshotId, package_name); - - int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode); - if (rc != 0) { - return error(rc, "Failed to create folder " + to); - } - - rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */); - if (rc != 0) { - return error(rc, "Failed clearing existing snapshot " + rollback_package_path); - } - - rc = copy_directory_recursive(from.c_str(), to.c_str()); - if (rc != 0) { - res = error(rc, "Failed copying " + from + " to " + to); - clear_de_on_exit = true; - return res; - } - } - if (storageFlags & FLAG_STORAGE_CE) { auto from = create_data_user_ce_package_path(volume_uuid, user, package_name); auto to = create_data_misc_ce_rollback_path(volume_uuid, user, snapshotId); -- cgit v1.2.3-59-g8ed1b From 995b24692cd557b71b46be0a402ad8b0ee5c1a09 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 6 Oct 2020 13:42:41 +0800 Subject: Fixes De data is not backed up correctly when user locked (1/n) https://android.googlesource.com/platform/frameworks/native/+/fda19ecdd6df8f43d3368781a633792723fb9965/cmds/installd/InstalldNativeService.cpp#933 When an APK is stage installed, snapshotAppData(..., FLAG_STORAGE_DE) is called before user unlocked. The function bails out early at #937 because the folder is still encrypted. We should move the if block at #958 above so we can back up De data correctly even when Ce directories not ready. Note we have to check if the De folder exists before making a copy. (Cherry-picked from 1d643c77d3fcb7dfe0bf4dabed2964bbfca04c6f) Bug: 169594054 Test: atest StagedRollbackTest Merged-In: I2ca810bd9495de3bed58378a41b47863c6e8f8dd Change-Id: I2ca810bd9495de3bed58378a41b47863c6e8f8dd --- cmds/installd/InstalldNativeService.cpp | 51 +++++++++++++++++---------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index 34727270e4..856b545c7c 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -876,6 +876,33 @@ binder::Status InstalldNativeService::snapshotAppData( auto scope_guard = android::base::make_scope_guard(deleter); + if (storageFlags & FLAG_STORAGE_DE) { + auto from = create_data_user_de_package_path(volume_uuid, user, package_name); + auto to = create_data_misc_de_rollback_path(volume_uuid, user, snapshotId); + auto rollback_package_path = create_data_misc_de_rollback_package_path(volume_uuid, user, + snapshotId, package_name); + + int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode); + if (rc != 0) { + return error(rc, "Failed to create folder " + to); + } + + rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */); + if (rc != 0) { + return error(rc, "Failed clearing existing snapshot " + rollback_package_path); + } + + // Check if we have data to copy. + if (access(from.c_str(), F_OK) == 0) { + rc = copy_directory_recursive(from.c_str(), to.c_str()); + } + if (rc != 0) { + res = error(rc, "Failed copying " + from + " to " + to); + clear_de_on_exit = true; + return res; + } + } + // The app may not have any data at all, in which case it's OK to skip here. auto from_ce = create_data_user_ce_package_path(volume_uuid, user, package_name); if (access(from_ce.c_str(), F_OK) != 0) { @@ -901,30 +928,6 @@ binder::Status InstalldNativeService::snapshotAppData( LOG(WARNING) << "Failed to clear code_cache of app " << packageName; } - if (storageFlags & FLAG_STORAGE_DE) { - auto from = create_data_user_de_package_path(volume_uuid, user, package_name); - auto to = create_data_misc_de_rollback_path(volume_uuid, user, snapshotId); - auto rollback_package_path = create_data_misc_de_rollback_package_path(volume_uuid, user, - snapshotId, package_name); - - int rc = create_dir_if_needed(to.c_str(), kRollbackFolderMode); - if (rc != 0) { - return error(rc, "Failed to create folder " + to); - } - - rc = delete_dir_contents(rollback_package_path, true /* ignore_if_missing */); - if (rc != 0) { - return error(rc, "Failed clearing existing snapshot " + rollback_package_path); - } - - rc = copy_directory_recursive(from.c_str(), to.c_str()); - if (rc != 0) { - res = error(rc, "Failed copying " + from + " to " + to); - clear_de_on_exit = true; - return res; - } - } - if (storageFlags & FLAG_STORAGE_CE) { auto from = create_data_user_ce_package_path(volume_uuid, user, package_name); auto to = create_data_misc_ce_rollback_path(volume_uuid, user, snapshotId); -- cgit v1.2.3-59-g8ed1b From 7c227cc333b85938a1ad0f860655bb83567ca755 Mon Sep 17 00:00:00 2001 From: Jon Spivack Date: Tue, 27 Oct 2020 19:29:14 -0700 Subject: libbinder: Add ClientCounterCallbackImpl to LazyServiceRegistrar This extra layer of indirection below ClientCounterCallback fixes a shared pointer ownership issue between LazyServiceRegistrar and ServiceManager. It also allows for implementation changes (like this one) without changing headers and breaking VNDK. Bug: 170212632 Test: Manual (Went through reproduction steps in bug on cf_x86_phone-userdebug) Test: atest aidl_lazy_test Change-Id: I4164a6d44e567c752726953e85aee0e91c6b525e Merged-In: I4164a6d44e567c752726953e85aee0e91c6b525e --- libs/binder/LazyServiceRegistrar.cpp | 46 +++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index 6f49aa1607..f2c5139b56 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -29,16 +29,12 @@ namespace internal { using AidlServiceManager = android::os::IServiceManager; -class ClientCounterCallback : public ::android::os::BnClientCallback { +class ClientCounterCallbackImpl : public ::android::os::BnClientCallback { public: - ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {} + ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {} bool registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags); - - /** - * Set a flag to prevent services from automatically shutting down - */ void forcePersist(bool persist); protected: @@ -69,7 +65,23 @@ private: bool mForcePersist; }; -bool ClientCounterCallback::registerService(const sp& service, const std::string& name, +class ClientCounterCallback { +public: + ClientCounterCallback(); + + bool registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags); + + /** + * Set a flag to prevent services from automatically shutting down + */ + void forcePersist(bool persist); + +private: + sp mImpl; +}; + +bool ClientCounterCallbackImpl::registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags) { auto manager = interface_cast(asBinder(defaultServiceManager())); @@ -95,7 +107,7 @@ bool ClientCounterCallback::registerService(const sp& service, const st return true; } -void ClientCounterCallback::forcePersist(bool persist) { +void ClientCounterCallbackImpl::forcePersist(bool persist) { mForcePersist = persist; if(!mForcePersist) { // Attempt a shutdown in case the number of clients hit 0 while the flag was on @@ -107,7 +119,7 @@ void ClientCounterCallback::forcePersist(bool persist) { * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple * invocations could occur on different threads however. */ -Status ClientCounterCallback::onClients(const sp& service, bool clients) { +Status ClientCounterCallbackImpl::onClients(const sp& service, bool clients) { if (clients) { mNumConnectedServices++; } else { @@ -122,7 +134,7 @@ Status ClientCounterCallback::onClients(const sp& service, bool clients return Status::ok(); } -void ClientCounterCallback::tryShutdown() { +void ClientCounterCallbackImpl::tryShutdown() { if(mNumConnectedServices > 0) { // Should only shut down if there are no clients return; @@ -143,7 +155,6 @@ void ClientCounterCallback::tryShutdown() { bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk(); - if (!success) { ALOGI("Failed to unregister service %s", entry.first.c_str()); break; @@ -168,6 +179,19 @@ void ClientCounterCallback::tryShutdown() { } } +ClientCounterCallback::ClientCounterCallback() { + mImpl = new ClientCounterCallbackImpl(); +} + +bool ClientCounterCallback::registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags) { + return mImpl->registerService(service, name, allowIsolated, dumpFlags); +} + +void ClientCounterCallback::forcePersist(bool persist) { + mImpl->forcePersist(persist); +} + } // namespace internal LazyServiceRegistrar::LazyServiceRegistrar() { -- cgit v1.2.3-59-g8ed1b From 143a10dba6f492f0307b54f8d5e1e7e26bc887f1 Mon Sep 17 00:00:00 2001 From: Jon Spivack Date: Tue, 27 Oct 2020 19:29:14 -0700 Subject: libbinder: Add ClientCounterCallbackImpl to LazyServiceRegistrar This extra layer of indirection below ClientCounterCallback fixes a shared pointer ownership issue between LazyServiceRegistrar and ServiceManager. It also allows for implementation changes (like this one) without changing headers and breaking VNDK. Bug: 170212632 Test: Manual (Went through reproduction steps in bug on rvc-dev) Test: atest aidl_lazy_test Change-Id: I4164a6d44e567c752726953e85aee0e91c6b525e (cherry picked from commit e31d33a397507069a4333396f999bb33a1a9fb70) Merged-In: I4164a6d44e567c752726953e85aee0e91c6b525e --- libs/binder/LazyServiceRegistrar.cpp | 50 ++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 13 deletions(-) diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index 325e204203..2e15e50e94 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -29,16 +29,12 @@ namespace internal { using AidlServiceManager = android::os::IServiceManager; -class ClientCounterCallback : public ::android::os::BnClientCallback { +class ClientCounterCallbackImpl : public ::android::os::BnClientCallback { public: - ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {} + ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {} bool registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags); - - /** - * Set a flag to prevent services from automatically shutting down - */ void forcePersist(bool persist); protected: @@ -75,7 +71,23 @@ private: bool mForcePersist; }; -bool ClientCounterCallback::registerService(const sp& service, const std::string& name, +class ClientCounterCallback { +public: + ClientCounterCallback(); + + bool registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags); + + /** + * Set a flag to prevent services from automatically shutting down + */ + void forcePersist(bool persist); + +private: + sp mImpl; +}; + +bool ClientCounterCallbackImpl::registerService(const sp& service, const std::string& name, bool allowIsolated, int dumpFlags) { auto manager = interface_cast(asBinder(defaultServiceManager())); @@ -89,7 +101,7 @@ bool ClientCounterCallback::registerService(const sp& service, const st } if (!reRegister) { - if (!manager->registerClientCallback(name, service, this).isOk()) { + if(!manager->registerClientCallback(name, service, this).isOk()) { ALOGE("Failed to add client callback for service %s", name.c_str()); return false; } @@ -105,7 +117,7 @@ bool ClientCounterCallback::registerService(const sp& service, const st return true; } -std::map::iterator ClientCounterCallback::assertRegisteredService(const sp& service) { +std::map::iterator ClientCounterCallbackImpl::assertRegisteredService(const sp& service) { LOG_ALWAYS_FATAL_IF(service == nullptr, "Got onClients callback for null service"); for (auto it = mRegisteredServices.begin(); it != mRegisteredServices.end(); ++it) { auto const& [name, registered] = *it; @@ -117,7 +129,7 @@ std::map::iterator ClientCounterCal __builtin_unreachable(); } -void ClientCounterCallback::forcePersist(bool persist) { +void ClientCounterCallbackImpl::forcePersist(bool persist) { mForcePersist = persist; if(!mForcePersist) { // Attempt a shutdown in case the number of clients hit 0 while the flag was on @@ -129,7 +141,7 @@ void ClientCounterCallback::forcePersist(bool persist) { * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple * invocations could occur on different threads however. */ -Status ClientCounterCallback::onClients(const sp& service, bool clients) { +Status ClientCounterCallbackImpl::onClients(const sp& service, bool clients) { auto & [name, registered] = *assertRegisteredService(service); if (registered.clients == clients) { LOG_ALWAYS_FATAL("Process already thought %s had clients: %d but servicemanager has " @@ -154,7 +166,7 @@ Status ClientCounterCallback::onClients(const sp& service, bool clients return Status::ok(); } -void ClientCounterCallback::tryShutdown() { +void ClientCounterCallbackImpl::tryShutdown() { if(mNumConnectedServices > 0) { // Should only shut down if there are no clients return; @@ -175,7 +187,6 @@ void ClientCounterCallback::tryShutdown() { bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk(); - if (!success) { ALOGI("Failed to unregister service %s", entry.first.c_str()); break; @@ -200,6 +211,19 @@ void ClientCounterCallback::tryShutdown() { } } +ClientCounterCallback::ClientCounterCallback() { + mImpl = sp::make(); +} + +bool ClientCounterCallback::registerService(const sp& service, const std::string& name, + bool allowIsolated, int dumpFlags) { + return mImpl->registerService(service, name, allowIsolated, dumpFlags); +} + +void ClientCounterCallback::forcePersist(bool persist) { + mImpl->forcePersist(persist); +} + } // namespace internal LazyServiceRegistrar::LazyServiceRegistrar() { -- cgit v1.2.3-59-g8ed1b From 2d5e05b44248e445af986ca8410c6e98c6ba3533 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Thu, 26 Nov 2020 12:12:45 +0100 Subject: Don't return a reference in FindQuotaDeviceForUuid. Since the object could be destroyed after we return it. Bug: 169421939 Test: N/A Change-Id: Ib2f68f87b5e04d59f825b2b1f48de39ca007452f --- cmds/installd/QuotaUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmds/installd/QuotaUtils.cpp b/cmds/installd/QuotaUtils.cpp index e0802911ca..60271392e9 100644 --- a/cmds/installd/QuotaUtils.cpp +++ b/cmds/installd/QuotaUtils.cpp @@ -35,7 +35,7 @@ std::recursive_mutex mMountsLock; /* Map of all quota mounts from target to source */ std::unordered_map mQuotaReverseMounts; -std::string& FindQuotaDeviceForUuid(const std::string& uuid) { +std::string FindQuotaDeviceForUuid(const std::string& uuid) { std::lock_guard lock(mMountsLock); auto path = create_data_path(uuid.empty() ? nullptr : uuid.c_str()); return mQuotaReverseMounts[path]; -- cgit v1.2.3-59-g8ed1b From 58f5cfa56d5282e69a7580dc4bb97603c409f003 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Wed, 18 Nov 2020 00:32:42 +0000 Subject: libbinder: check null bytes in readString*Inplace This is entirely defensive, since the only real guarantee we have here from these APIs is that a buffer of a given length is available. However, since we write 0's here, presumably to guard against people assuming these are null-terminated strings, we might as well enforce that they are actually null terminated. Bug: 172655291 Test: binderParcelTest (added in newer CL) Change-Id: Ie879112540155f6a93b97aeaf3d41ed8ba4ae79f Merged-In: Ie879112540155f6a93b97aeaf3d41ed8ba4ae79f (cherry picked from commit 51e02b16c397c44ddf81a0736cf6045cd4c44128) --- libs/binder/Parcel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 9642a87f4e..1f7d27e0e9 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -1869,7 +1869,7 @@ const char* Parcel::readString8Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char* str = (const char*)readInplace(size+1); - if (str != nullptr) { + if (str != nullptr && str[size] == '\0') { return str; } } @@ -1929,7 +1929,7 @@ const char16_t* Parcel::readString16Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char16_t* str = (const char16_t*)readInplace((size+1)*sizeof(char16_t)); - if (str != nullptr) { + if (str != nullptr && str[size] == u'\0') { return str; } } -- cgit v1.2.3-59-g8ed1b From 61d0f84881cfc1bbac513ccd156c56603a48cc90 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 4 Dec 2020 21:13:03 +0000 Subject: libbinder: readString*Inplace SafetyNet (II) SafetyNet logs (this time for failure case, instead of success case). Bug: 172655291 Test: adb logcat -b events | grep snet # exactly one occurance w/ repro (c/p'd from 34af0637666f43ae62040ad1bad76468423feba2) Merged-In: I75ace071693c0a4579ed9477f7b9212a6e27c36d Change-Id: I75ace071693c0a4579ed9477f7b9212a6e27c36d --- libs/binder/Parcel.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 1f7d27e0e9..b7ad660317 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -1869,8 +1869,11 @@ const char* Parcel::readString8Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char* str = (const char*)readInplace(size+1); - if (str != nullptr && str[size] == '\0') { - return str; + if (str != nullptr) { + if (str[size] == '\0') { + return str; + } + android_errorWriteLog(0x534e4554, "172655291"); } } *outLen = 0; @@ -1929,8 +1932,11 @@ const char16_t* Parcel::readString16Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char16_t* str = (const char16_t*)readInplace((size+1)*sizeof(char16_t)); - if (str != nullptr && str[size] == u'\0') { - return str; + if (str != nullptr) { + if (str[size] == u'\0') { + return str; + } + android_errorWriteLog(0x534e4554, "172655291"); } } *outLen = 0; -- cgit v1.2.3-59-g8ed1b From 58fbf9fb2c82ee09b0d572e0936b8370c4d35aef Mon Sep 17 00:00:00 2001 From: Denis Hsu Date: Wed, 17 Jun 2020 10:17:30 +0800 Subject: SF: update mInputFlinger on main thread mInputFlinger can be accessed by bootFinished() and updateInputFlinger(). These function can run in different thread at the same time, but mInputFlinger was not protected. update mInputFlinger on main thread Fixes: 157871763 Fixes: 147009853 Fixes: 169256435 Test: adb shell stop / start Test: boot ok Test: Race was only reproducible in monkey tests Change-Id: I4d87e90793a88a646aaa1ae5806f118f1ae51e30 Merged-In: I4d87e90793a88a646aaa1ae5806f118f1ae51e30 --- services/surfaceflinger/SurfaceFlinger.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 1a25b8230e..bc134cb158 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -538,13 +538,6 @@ void SurfaceFlinger::bootFinished() if (window != 0) { window->linkToDeath(static_cast(this)); } - sp input(defaultServiceManager()->getService( - String16("inputflinger"))); - if (input == nullptr) { - ALOGE("Failed to link to input service"); - } else { - mInputFlinger = interface_cast(input); - } if (mVrFlinger) { mVrFlinger->OnBootFinished(); @@ -559,7 +552,15 @@ void SurfaceFlinger::bootFinished() LOG_EVENT_LONG(LOGTAG_SF_STOP_BOOTANIM, ns2ms(systemTime(SYSTEM_TIME_MONOTONIC))); - postMessageAsync(new LambdaMessage([this]() NO_THREAD_SAFETY_ANALYSIS { + sp input(defaultServiceManager()->getService(String16("inputflinger"))); + + postMessageAsync(new LambdaMessage([=]() NO_THREAD_SAFETY_ANALYSIS { + if (input == nullptr) { + ALOGE("Failed to link to input service"); + } else { + mInputFlinger = interface_cast(input); + } + readPersistentProperties(); mBootStage = BootStage::FINISHED; -- cgit v1.2.3-59-g8ed1b From 4240670c67e5c6a6865b69a745fbf8e011727db9 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Wed, 14 Oct 2020 08:42:26 -0700 Subject: Vulkan: load built-in driver into default namespace as a fallback There isn't sphal in vendor config because the default has the same access there. This change allows vendor processes to load Vulkan driver into the default namespace. Bug: 170258171 Test: Vulkan driver can be loaded into vendor processes (cherry picked from commit ecf8a24fc10398afc8bbec6e94bab25fc605fb4d) Merged-In: If58493e6954e4e8d2309aaca392fcdffea9c6b9a Change-Id: I96efd5234fdcdac3bfcd48b1c3f1eb309db1bb00 --- vulkan/libvulkan/Android.bp | 2 +- vulkan/libvulkan/driver.cpp | 60 ++++++++++++++++++++------------------------- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/vulkan/libvulkan/Android.bp b/vulkan/libvulkan/Android.bp index 921b095726..1d29bab355 100644 --- a/vulkan/libvulkan/Android.bp +++ b/vulkan/libvulkan/Android.bp @@ -90,7 +90,6 @@ cc_library_shared { "libhardware", "libsync", "libbase", - "libdl_android", "libhidlbase", "liblog", "libui", @@ -101,6 +100,7 @@ cc_library_shared { "libnativebridge_lazy", "libnativeloader_lazy", "libnativewindow", + "libvndksupport", "android.hardware.graphics.common@1.0", "libSurfaceFlingerProp", ], diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index c63fdf5969..5d4717200d 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -28,20 +28,17 @@ #include #include #include -#include #include #include -#include #include #include #include +#include #include #include #include #include -#include -#include #include #include "stubhal.h" @@ -151,19 +148,11 @@ class CreateInfoWrapper { Hal Hal::hal_; -void* LoadLibrary(const android_dlextinfo& dlextinfo, - const std::string_view subname) { - ATRACE_CALL(); - - std::stringstream ss; - ss << "vulkan." << subname << ".so"; - return android_dlopen_ext(ss.str().c_str(), RTLD_LOCAL | RTLD_NOW, &dlextinfo); -} - const std::array HAL_SUBNAME_KEY_PROPERTIES = {{ - "ro.hardware.vulkan", - "ro.board.platform", + "ro.hardware." HWVULKAN_HARDWARE_MODULE_ID, + "ro.board.platform" }}; +constexpr int LIB_DL_FLAGS = RTLD_LOCAL | RTLD_NOW; // LoadDriver returns: // * 0 when succeed, or @@ -174,23 +163,30 @@ int LoadDriver(android_namespace_t* library_namespace, const hwvulkan_module_t** module) { ATRACE_CALL(); - const android_dlextinfo dlextinfo = { - .flags = ANDROID_DLEXT_USE_NAMESPACE, - .library_namespace = library_namespace, - }; void* so = nullptr; - char prop[PROPERTY_VALUE_MAX]; for (auto key : HAL_SUBNAME_KEY_PROPERTIES) { - int prop_len = property_get(key, prop, nullptr); - if (prop_len > 0 && prop_len <= UINT_MAX) { - std::string_view lib_name(prop, static_cast(prop_len)); - so = LoadLibrary(dlextinfo, lib_name); - if (so) - break; + std::string lib_name = android::base::GetProperty(key, ""); + if (lib_name.empty()) + continue; + + lib_name = "vulkan." + lib_name + ".so"; + if (library_namespace) { + // load updated driver + const android_dlextinfo dlextinfo = { + .flags = ANDROID_DLEXT_USE_NAMESPACE, + .library_namespace = library_namespace, + }; + so = android_dlopen_ext(lib_name.c_str(), LIB_DL_FLAGS, &dlextinfo); + } else { + // load built-in driver + so = android_load_sphal_library(lib_name.c_str(), LIB_DL_FLAGS); } + if (so) + break; } - if (!so) + if (!so) { return -ENOENT; + } auto hmi = static_cast(dlsym(so, HAL_MODULE_INFO_SYM_AS_STR)); if (!hmi) { @@ -211,12 +207,9 @@ int LoadDriver(android_namespace_t* library_namespace, int LoadBuiltinDriver(const hwvulkan_module_t** module) { ATRACE_CALL(); - auto ns = android_get_exported_namespace("sphal"); - if (!ns) - return -ENOENT; android::GraphicsEnv::getInstance().setDriverToLoad( android::GpuStatsInfo::Driver::VULKAN); - return LoadDriver(ns, module); + return LoadDriver(nullptr, module); } int LoadUpdatedDriver(const hwvulkan_module_t** module) { @@ -238,7 +231,6 @@ int LoadUpdatedDriver(const hwvulkan_module_t** module) { bool Hal::Open() { ATRACE_CALL(); - const nsecs_t openTime = systemTime(); ALOG_ASSERT(!hal_.dev_, "OpenHAL called more than once"); @@ -256,16 +248,16 @@ bool Hal::Open() { if (result != 0) { android::GraphicsEnv::getInstance().setDriverLoaded( android::GpuStatsInfo::Api::API_VK, false, systemTime() - openTime); - ALOGV("unable to load Vulkan HAL, using stub HAL (result=%d)", result); return true; } - hwvulkan_device_t* device; ATRACE_BEGIN("hwvulkan module open"); result = module->common.methods->open(&module->common, HWVULKAN_DEVICE_0, reinterpret_cast(&device)); + + ATRACE_END(); if (result != 0) { android::GraphicsEnv::getInstance().setDriverLoaded( -- cgit v1.2.3-59-g8ed1b From d10af92a7b83d252038944cad6014a00bfa7ca25 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Wed, 14 Oct 2020 08:42:26 -0700 Subject: Vulkan: load built-in driver into default namespace as a fallback There isn't sphal in vendor config because the default has the same access there. This change allows vendor processes to load Vulkan driver into the default namespace. Bug: 170258171 Test: Vulkan driver can be loaded into vendor processes (cherry picked from commit ecf8a24fc10398afc8bbec6e94bab25fc605fb4d) Merged-In: I96efd5234fdcdac3bfcd48b1c3f1eb309db1bb00 Merged-In: If58493e6954e4e8d2309aaca392fcdffea9c6b9a Change-Id: I96efd5234fdcdac3bfcd48b1c3f1eb309db1bb00 --- vulkan/libvulkan/Android.bp | 2 +- vulkan/libvulkan/driver.cpp | 58 +++++++++++++++++++-------------------------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/vulkan/libvulkan/Android.bp b/vulkan/libvulkan/Android.bp index f69de1f324..aa8040b8f2 100644 --- a/vulkan/libvulkan/Android.bp +++ b/vulkan/libvulkan/Android.bp @@ -89,7 +89,6 @@ cc_library_shared { "libhardware", "libsync", "libbase", - "libdl_android", "libhidlbase", "liblog", "libui", @@ -100,6 +99,7 @@ cc_library_shared { "libnativebridge_lazy", "libnativeloader_lazy", "libnativewindow", + "libvndksupport", "android.hardware.graphics.common@1.0", "libSurfaceFlingerProp", ], diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 7bcb2c1441..f840561292 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -28,20 +28,17 @@ #include #include #include -#include #include #include -#include #include #include #include +#include #include #include #include #include -#include -#include #include #include "stubhal.h" @@ -151,19 +148,11 @@ class CreateInfoWrapper { Hal Hal::hal_; -void* LoadLibrary(const android_dlextinfo& dlextinfo, - const std::string_view subname) { - ATRACE_CALL(); - - std::stringstream ss; - ss << "vulkan." << subname << ".so"; - return android_dlopen_ext(ss.str().c_str(), RTLD_LOCAL | RTLD_NOW, &dlextinfo); -} - const std::array HAL_SUBNAME_KEY_PROPERTIES = {{ "ro.hardware." HWVULKAN_HARDWARE_MODULE_ID, - "ro.board.platform", + "ro.board.platform" }}; +constexpr int LIB_DL_FLAGS = RTLD_LOCAL | RTLD_NOW; // LoadDriver returns: // * 0 when succeed, or @@ -174,23 +163,30 @@ int LoadDriver(android_namespace_t* library_namespace, const hwvulkan_module_t** module) { ATRACE_CALL(); - const android_dlextinfo dlextinfo = { - .flags = ANDROID_DLEXT_USE_NAMESPACE, - .library_namespace = library_namespace, - }; void* so = nullptr; - char prop[PROPERTY_VALUE_MAX]; for (auto key : HAL_SUBNAME_KEY_PROPERTIES) { - int prop_len = property_get(key, prop, nullptr); - if (prop_len > 0 && prop_len <= UINT_MAX) { - std::string_view lib_name(prop, static_cast(prop_len)); - so = LoadLibrary(dlextinfo, lib_name); - if (so) - break; + std::string lib_name = android::base::GetProperty(key, ""); + if (lib_name.empty()) + continue; + + lib_name = "vulkan." + lib_name + ".so"; + if (library_namespace) { + // load updated driver + const android_dlextinfo dlextinfo = { + .flags = ANDROID_DLEXT_USE_NAMESPACE, + .library_namespace = library_namespace, + }; + so = android_dlopen_ext(lib_name.c_str(), LIB_DL_FLAGS, &dlextinfo); + } else { + // load built-in driver + so = android_load_sphal_library(lib_name.c_str(), LIB_DL_FLAGS); } + if (so) + break; } - if (!so) + if (!so) { return -ENOENT; + } auto hmi = static_cast(dlsym(so, HAL_MODULE_INFO_SYM_AS_STR)); if (!hmi) { @@ -211,12 +207,9 @@ int LoadDriver(android_namespace_t* library_namespace, int LoadBuiltinDriver(const hwvulkan_module_t** module) { ATRACE_CALL(); - auto ns = android_get_exported_namespace("sphal"); - if (!ns) - return -ENOENT; android::GraphicsEnv::getInstance().setDriverToLoad( android::GpuStatsInfo::Driver::VULKAN); - return LoadDriver(ns, module); + return LoadDriver(nullptr, module); } int LoadUpdatedDriver(const hwvulkan_module_t** module) { @@ -238,7 +231,6 @@ int LoadUpdatedDriver(const hwvulkan_module_t** module) { bool Hal::Open() { ATRACE_CALL(); - const nsecs_t openTime = systemTime(); ALOG_ASSERT(!hal_.dev_, "OpenHAL called more than once"); @@ -256,16 +248,16 @@ bool Hal::Open() { if (result != 0) { android::GraphicsEnv::getInstance().setDriverLoaded( android::GpuStatsInfo::Api::API_VK, false, systemTime() - openTime); - ALOGV("unable to load Vulkan HAL, using stub HAL (result=%d)", result); return true; } - hwvulkan_device_t* device; ATRACE_BEGIN("hwvulkan module open"); result = module->common.methods->open(&module->common, HWVULKAN_DEVICE_0, reinterpret_cast(&device)); + + ATRACE_END(); if (result != 0) { android::GraphicsEnv::getInstance().setDriverLoaded( -- cgit v1.2.3-59-g8ed1b From 844e19b8566129110ef8a59dd71cd24ef1b26675 Mon Sep 17 00:00:00 2001 From: Amos Bianchi Date: Wed, 23 Dec 2020 09:57:02 -0800 Subject: Add active services count callback to lazy services. Additionally, expose methods to tryUnregister/reRegister services. Bug: 176239128 Test: test aidl_lazy_test Change-Id: I9aa3c9b681bd340ca340fe7ed818ba3533678af2 Merged-In: I9aa3c9b681bd340ca340fe7ed818ba3533678af2 (cherry picked from commit 2055e9b48d273e0e8997ce4745a00ac6a24b09c6) --- libs/binder/LazyServiceRegistrar.cpp | 163 ++++++++++++++++------ libs/binder/include/binder/LazyServiceRegistrar.h | 36 +++++ 2 files changed, 156 insertions(+), 43 deletions(-) diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index f2c5139b56..bfd6942c3e 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include "log/log_main.h" #define LOG_TAG "AidlLazyServiceRegistrar" #include @@ -37,6 +38,13 @@ public: bool allowIsolated, int dumpFlags); void forcePersist(bool persist); + void setActiveServicesCountCallback(const std::function& + activeServicesCountCallback); + + bool tryUnregister(); + + void reRegister(); + protected: Status onClients(const sp& service, bool clients) override; @@ -48,6 +56,14 @@ private: void tryShutdown(); /** + * Try to shutdown the process, unless: + * - 'forcePersist' is 'true', or + * - The active services count callback returns 'true', or + * - Some services have clients. + */ + void maybeTryShutdown(); + + /* * Counter of the number of services that currently have at least one client. */ size_t mNumConnectedServices; @@ -56,6 +72,8 @@ private: sp service; bool allowIsolated; int dumpFlags; + + bool registered = true; }; /** * Map of registered names and services @@ -63,6 +81,9 @@ private: std::map mRegisteredServices; bool mForcePersist; + + // Callback used to report the number of services with clients + std::function mActiveServicesCountCallback; }; class ClientCounterCallback { @@ -77,6 +98,13 @@ public: */ void forcePersist(bool persist); + void setActiveServicesCountCallback(const std::function& + activeServicesCountCallback); + + bool tryUnregister(); + + void reRegister(); + private: sp mImpl; }; @@ -109,8 +137,60 @@ bool ClientCounterCallbackImpl::registerService(const sp& service, cons void ClientCounterCallbackImpl::forcePersist(bool persist) { mForcePersist = persist; - if(!mForcePersist) { + if (!mForcePersist && mNumConnectedServices == 0) { // Attempt a shutdown in case the number of clients hit 0 while the flag was on + maybeTryShutdown(); + } +} + +bool ClientCounterCallbackImpl::tryUnregister() { + auto manager = interface_cast(asBinder(defaultServiceManager())); + + for (auto& [name, entry] : mRegisteredServices) { + bool success = manager->tryUnregisterService(name, entry.service).isOk(); + + if (!success) { + ALOGI("Failed to unregister service %s", name.c_str()); + return false; + } + entry.registered = false; + } + + return true; +} + +void ClientCounterCallbackImpl::reRegister() { + for (auto& [name, entry] : mRegisteredServices) { + // re-register entry if not already registered + if (entry.registered) { + continue; + } + + if (!registerService(entry.service, name, entry.allowIsolated, + entry.dumpFlags)) { + // Must restart. Otherwise, clients will never be able to get a hold of this service. + LOG_ALWAYS_FATAL("Bad state: could not re-register services"); + } + + entry.registered = true; + } +} + +void ClientCounterCallbackImpl::maybeTryShutdown() { + if (mForcePersist) { + ALOGI("Shutdown prevented by forcePersist override flag."); + return; + } + + bool handledInCallback = false; + if (mActiveServicesCountCallback != nullptr) { + handledInCallback = mActiveServicesCountCallback(mNumConnectedServices); + } + + // If there is no callback defined or the callback did not handle this + // client count change event, try to shutdown the process if its services + // have no clients. + if (!handledInCallback && mNumConnectedServices == 0) { tryShutdown(); } } @@ -130,53 +210,24 @@ Status ClientCounterCallbackImpl::onClients(const sp& service, bool cli mNumConnectedServices, mRegisteredServices.size(), String8(service->getInterfaceDescriptor()).string(), clients); - tryShutdown(); + maybeTryShutdown(); return Status::ok(); } -void ClientCounterCallbackImpl::tryShutdown() { - if(mNumConnectedServices > 0) { - // Should only shut down if there are no clients - return; - } - - if(mForcePersist) { - ALOGI("Shutdown prevented by forcePersist override flag."); - return; - } - - ALOGI("Trying to shut down the service. No clients in use for any service in process."); - - auto manager = interface_cast(asBinder(defaultServiceManager())); + void ClientCounterCallbackImpl::tryShutdown() { + ALOGI("Trying to shut down the service. No clients in use for any service in process."); - auto unRegisterIt = mRegisteredServices.begin(); - for (; unRegisterIt != mRegisteredServices.end(); ++unRegisterIt) { - auto& entry = (*unRegisterIt); + if (tryUnregister()) { + ALOGI("Unregistered all clients and exiting"); + exit(EXIT_SUCCESS); + } - bool success = manager->tryUnregisterService(entry.first, entry.second.service).isOk(); - - if (!success) { - ALOGI("Failed to unregister service %s", entry.first.c_str()); - break; - } - } - - if (unRegisterIt == mRegisteredServices.end()) { - ALOGI("Unregistered all clients and exiting"); - exit(EXIT_SUCCESS); - } - - for (auto reRegisterIt = mRegisteredServices.begin(); reRegisterIt != unRegisterIt; - reRegisterIt++) { - auto& entry = (*reRegisterIt); + reRegister(); +} - // re-register entry - if (!registerService(entry.second.service, entry.first, entry.second.allowIsolated, - entry.second.dumpFlags)) { - // Must restart. Otherwise, clients will never be able to get a hold of this service. - ALOGE("Bad state: could not re-register services"); - } - } +void ClientCounterCallbackImpl::setActiveServicesCountCallback(const std::function& + activeServicesCountCallback) { + mActiveServicesCountCallback = activeServicesCountCallback; } ClientCounterCallback::ClientCounterCallback() { @@ -192,6 +243,19 @@ void ClientCounterCallback::forcePersist(bool persist) { mImpl->forcePersist(persist); } +void ClientCounterCallback::setActiveServicesCountCallback(const std::function& + activeServicesCountCallback) { + mImpl->setActiveServicesCountCallback(activeServicesCountCallback); +} + +bool ClientCounterCallback::tryUnregister() { + return mImpl->tryUnregister(); +} + +void ClientCounterCallback::reRegister() { + mImpl->reRegister(); +} + } // namespace internal LazyServiceRegistrar::LazyServiceRegistrar() { @@ -215,5 +279,18 @@ void LazyServiceRegistrar::forcePersist(bool persist) { mClientCC->forcePersist(persist); } +void LazyServiceRegistrar::setActiveServicesCountCallback(const std::function& + activeServicesCountCallback) { + mClientCC->setActiveServicesCountCallback(activeServicesCountCallback); +} + +bool LazyServiceRegistrar::tryUnregister() { + return mClientCC->tryUnregister(); +} + +void LazyServiceRegistrar::reRegister() { + mClientCC->reRegister(); +} + } // namespace hardware -} // namespace android \ No newline at end of file +} // namespace android diff --git a/libs/binder/include/binder/LazyServiceRegistrar.h b/libs/binder/include/binder/LazyServiceRegistrar.h index d18c88ec0e..73893c7dd0 100644 --- a/libs/binder/include/binder/LazyServiceRegistrar.h +++ b/libs/binder/include/binder/LazyServiceRegistrar.h @@ -16,6 +16,8 @@ #pragma once +#include + #include #include #include @@ -53,6 +55,40 @@ class LazyServiceRegistrar { */ void forcePersist(bool persist); + /** + * Set a callback that is executed when the total number of services with + * clients changes. + * The callback takes an argument, which is the number of registered + * lazy services for this process which have clients. + * + * Callback return value: + * - false: Default behavior for lazy services (shut down the process if there + * are no clients). + * - true: Don't shut down the process even if there are no clients. + * + * This callback gives a chance to: + * 1 - Perform some additional operations before exiting; + * 2 - Prevent the process from exiting by returning "true" from the + * callback. + * + * This method should be called before 'registerService' to avoid races. + */ + void setActiveServicesCountCallback(const std::function& + activeServicesCountCallback); + + /** + * Try to unregister all services previously registered with 'registerService'. + * Returns 'true' if successful. + */ + bool tryUnregister(); + + /** + * Re-register services that were unregistered by 'tryUnregister'. + * This method should be called in the case 'tryUnregister' fails + * (and should be called on the same thread). + */ + void reRegister(); + private: std::shared_ptr mClientCC; LazyServiceRegistrar(); -- cgit v1.2.3-59-g8ed1b From 3f796949940af3e99ccb9c404391ce7773a99b25 Mon Sep 17 00:00:00 2001 From: Amos Bianchi Date: Wed, 20 Jan 2021 16:06:56 -0800 Subject: Change argument of active services callback to bool. Instead of passing the number of active services, pass a bool that represents if there are clients. Bug: 176239128 Test: test aidl_lazy_test Change-Id: I8180547dfe4ebc11d5d7bb3a7306bc79f839d715 Merged-In: I8180547dfe4ebc11d5d7bb3a7306bc79f839d715 (cherry-picked from 1afb14e050360ece9f127bf27bb416cb274f0eed) --- libs/binder/LazyServiceRegistrar.cpp | 41 +++++++++++++---------- libs/binder/include/binder/LazyServiceRegistrar.h | 11 +++--- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp index bfd6942c3e..68ea8a6d79 100644 --- a/libs/binder/LazyServiceRegistrar.cpp +++ b/libs/binder/LazyServiceRegistrar.cpp @@ -38,8 +38,7 @@ public: bool allowIsolated, int dumpFlags); void forcePersist(bool persist); - void setActiveServicesCountCallback(const std::function& - activeServicesCountCallback); + void setActiveServicesCallback(const std::function& activeServicesCallback); bool tryUnregister(); @@ -68,6 +67,9 @@ private: */ size_t mNumConnectedServices; + // previous value passed to the active services callback + std::optional mPreviousHasClients; + struct Service { sp service; bool allowIsolated; @@ -82,8 +84,8 @@ private: bool mForcePersist; - // Callback used to report the number of services with clients - std::function mActiveServicesCountCallback; + // Callback used to report if there are services with clients + std::function mActiveServicesCallback; }; class ClientCounterCallback { @@ -98,8 +100,7 @@ public: */ void forcePersist(bool persist); - void setActiveServicesCountCallback(const std::function& - activeServicesCountCallback); + void setActiveServicesCallback(const std::function& activeServicesCallback); bool tryUnregister(); @@ -137,7 +138,7 @@ bool ClientCounterCallbackImpl::registerService(const sp& service, cons void ClientCounterCallbackImpl::forcePersist(bool persist) { mForcePersist = persist; - if (!mForcePersist && mNumConnectedServices == 0) { + if (!mForcePersist) { // Attempt a shutdown in case the number of clients hit 0 while the flag was on maybeTryShutdown(); } @@ -183,8 +184,12 @@ void ClientCounterCallbackImpl::maybeTryShutdown() { } bool handledInCallback = false; - if (mActiveServicesCountCallback != nullptr) { - handledInCallback = mActiveServicesCountCallback(mNumConnectedServices); + if (mActiveServicesCallback != nullptr) { + bool hasClients = mNumConnectedServices != 0; + if (hasClients != mPreviousHasClients) { + handledInCallback = mActiveServicesCallback(hasClients); + mPreviousHasClients = hasClients; + } } // If there is no callback defined or the callback did not handle this @@ -225,9 +230,9 @@ Status ClientCounterCallbackImpl::onClients(const sp& service, bool cli reRegister(); } -void ClientCounterCallbackImpl::setActiveServicesCountCallback(const std::function& - activeServicesCountCallback) { - mActiveServicesCountCallback = activeServicesCountCallback; +void ClientCounterCallbackImpl::setActiveServicesCallback(const std::function& + activeServicesCallback) { + mActiveServicesCallback = activeServicesCallback; } ClientCounterCallback::ClientCounterCallback() { @@ -243,9 +248,9 @@ void ClientCounterCallback::forcePersist(bool persist) { mImpl->forcePersist(persist); } -void ClientCounterCallback::setActiveServicesCountCallback(const std::function& - activeServicesCountCallback) { - mImpl->setActiveServicesCountCallback(activeServicesCountCallback); +void ClientCounterCallback::setActiveServicesCallback(const std::function& + activeServicesCallback) { + mImpl->setActiveServicesCallback(activeServicesCallback); } bool ClientCounterCallback::tryUnregister() { @@ -279,9 +284,9 @@ void LazyServiceRegistrar::forcePersist(bool persist) { mClientCC->forcePersist(persist); } -void LazyServiceRegistrar::setActiveServicesCountCallback(const std::function& - activeServicesCountCallback) { - mClientCC->setActiveServicesCountCallback(activeServicesCountCallback); +void LazyServiceRegistrar::setActiveServicesCallback(const std::function& + activeServicesCallback) { + mClientCC->setActiveServicesCallback(activeServicesCallback); } bool LazyServiceRegistrar::tryUnregister() { diff --git a/libs/binder/include/binder/LazyServiceRegistrar.h b/libs/binder/include/binder/LazyServiceRegistrar.h index 73893c7dd0..96597320f6 100644 --- a/libs/binder/include/binder/LazyServiceRegistrar.h +++ b/libs/binder/include/binder/LazyServiceRegistrar.h @@ -56,10 +56,10 @@ class LazyServiceRegistrar { void forcePersist(bool persist); /** - * Set a callback that is executed when the total number of services with - * clients changes. - * The callback takes an argument, which is the number of registered - * lazy services for this process which have clients. + * Set a callback that is invoked when the active service count (i.e. services with clients) + * registered with this process drops to zero (or becomes nonzero). + * The callback takes a boolean argument, which is 'true' if there is + * at least one service with clients. * * Callback return value: * - false: Default behavior for lazy services (shut down the process if there @@ -73,8 +73,7 @@ class LazyServiceRegistrar { * * This method should be called before 'registerService' to avoid races. */ - void setActiveServicesCountCallback(const std::function& - activeServicesCountCallback); + void setActiveServicesCallback(const std::function& activeServicesCallback); /** * Try to unregister all services previously registered with 'registerService'. -- cgit v1.2.3-59-g8ed1b