diff options
| -rw-r--r-- | libs/binder/LazyServiceRegistrar.cpp | 50 | ||||
| -rw-r--r-- | libs/binder/ndk/ibinder.cpp | 2 | ||||
| -rw-r--r-- | libs/binder/ndk/ibinder_internal.h | 10 | ||||
| -rw-r--r-- | libs/binder/ndk/tests/iface.cpp | 12 | ||||
| -rw-r--r-- | libs/binder/ndk/tests/include/iface/iface.h | 3 | ||||
| -rw-r--r-- | libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp | 20 | ||||
| -rw-r--r-- | services/sensorservice/SensorEventConnection.cpp | 28 | ||||
| -rw-r--r-- | services/sensorservice/SensorEventConnection.h | 5 |
8 files changed, 65 insertions, 65 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<IBinder>& 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<IBinder>& service, const std::string& name, +class ClientCounterCallback { +public: + ClientCounterCallback(); + + bool registerService(const sp<IBinder>& 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<ClientCounterCallbackImpl> mImpl; +}; + +bool ClientCounterCallbackImpl::registerService(const sp<IBinder>& service, const std::string& name, bool allowIsolated, int dumpFlags) { auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager())); @@ -89,7 +101,7 @@ bool ClientCounterCallback::registerService(const sp<IBinder>& 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<IBinder>& service, const st return true; } -std::map<std::string, ClientCounterCallback::Service>::iterator ClientCounterCallback::assertRegisteredService(const sp<IBinder>& service) { +std::map<std::string, ClientCounterCallbackImpl::Service>::iterator ClientCounterCallbackImpl::assertRegisteredService(const sp<IBinder>& 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<std::string, ClientCounterCallback::Service>::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<IBinder>& service, bool clients) { +Status ClientCounterCallbackImpl::onClients(const sp<IBinder>& 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<IBinder>& 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<ClientCounterCallbackImpl>::make(); +} + +bool ClientCounterCallback::registerService(const sp<IBinder>& 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() { diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp index 5e2e1bd166..f0d93c3f0c 100644 --- a/libs/binder/ndk/ibinder.cpp +++ b/libs/binder/ndk/ibinder.cpp @@ -181,7 +181,7 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce binder_status_t status = getClass()->onTransact(this, code, &in, &out); return PruneStatusT(status); - } else if (code == SHELL_COMMAND_TRANSACTION && getClass()->handleShellCommand != nullptr) { + } else if (code == SHELL_COMMAND_TRANSACTION) { int in = data.readFileDescriptor(); int out = data.readFileDescriptor(); int err = data.readFileDescriptor(); diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h index 6236e81a78..25caf81971 100644 --- a/libs/binder/ndk/ibinder_internal.h +++ b/libs/binder/ndk/ibinder_internal.h @@ -115,13 +115,13 @@ struct AIBinder_Class { const ::android::String16& getInterfaceDescriptor() const { return mInterfaceDescriptor; } // required to be non-null, implemented for every class - const AIBinder_Class_onCreate onCreate = nullptr; - const AIBinder_Class_onDestroy onDestroy = nullptr; - const AIBinder_Class_onTransact onTransact = nullptr; + const AIBinder_Class_onCreate onCreate; + const AIBinder_Class_onDestroy onDestroy; + const AIBinder_Class_onTransact onTransact; // optional methods for a class - AIBinder_onDump onDump = nullptr; - AIBinder_handleShellCommand handleShellCommand = nullptr; + AIBinder_onDump onDump; + AIBinder_handleShellCommand handleShellCommand; private: // This must be a String16 since BBinder virtual getInterfaceDescriptor returns a reference to diff --git a/libs/binder/ndk/tests/iface.cpp b/libs/binder/ndk/tests/iface.cpp index a5889856fc..64832f3081 100644 --- a/libs/binder/ndk/tests/iface.cpp +++ b/libs/binder/ndk/tests/iface.cpp @@ -118,7 +118,7 @@ IFoo::~IFoo() { AIBinder_Weak_delete(mWeakBinder); } -AIBinder* IFoo::getBinder() { +binder_status_t IFoo::addService(const char* instance) { AIBinder* binder = nullptr; if (mWeakBinder != nullptr) { @@ -132,18 +132,8 @@ AIBinder* IFoo::getBinder() { AIBinder_Weak_delete(mWeakBinder); } mWeakBinder = AIBinder_Weak_new(binder); - - // WARNING: it is important that this class does not implement debug or - // shell functions because it does not use special C++ wrapper - // functions, and so this is how we test those functions. } - return binder; -} - -binder_status_t IFoo::addService(const char* instance) { - AIBinder* binder = getBinder(); - binder_status_t status = AServiceManager_addService(binder, instance); // Strong references we care about kept by remote process AIBinder_decStrong(binder); diff --git a/libs/binder/ndk/tests/include/iface/iface.h b/libs/binder/ndk/tests/include/iface/iface.h index d9dd64b8a6..cdf5493216 100644 --- a/libs/binder/ndk/tests/include/iface/iface.h +++ b/libs/binder/ndk/tests/include/iface/iface.h @@ -30,9 +30,6 @@ class IFoo : public virtual ::android::RefBase { static AIBinder_Class* kClass; - // binder representing this interface with one reference count - AIBinder* getBinder(); - // Takes ownership of IFoo binder_status_t addService(const char* instance); static ::android::sp<IFoo> getService(const char* instance, AIBinder** outBinder = nullptr); diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp index f84d9d3746..8b6414cebf 100644 --- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp +++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp @@ -185,26 +185,6 @@ TEST(NdkBinder, CheckServiceThatDoesExist) { AIBinder_decStrong(binder); } -TEST(NdkBinder, UnimplementedDump) { - sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName); - ASSERT_NE(foo, nullptr); - AIBinder* binder = foo->getBinder(); - EXPECT_EQ(OK, AIBinder_dump(binder, STDOUT_FILENO, nullptr, 0)); - AIBinder_decStrong(binder); -} - -TEST(NdkBinder, UnimplementedShell) { - // libbinder_ndk doesn't support calling shell, so we are calling from the - // libbinder across processes to the NDK service which doesn't implement - // shell - static const sp<android::IServiceManager> sm(android::defaultServiceManager()); - sp<IBinder> testService = sm->getService(String16(IFoo::kSomeInstanceName)); - - Vector<String16> argsVec; - EXPECT_EQ(OK, IBinder::shellCommand(testService, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO, - argsVec, nullptr, nullptr)); -} - TEST(NdkBinder, DoubleNumber) { sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName); ASSERT_NE(foo, nullptr); 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 <log/log.h> #include <sys/socket.h> #include <utils/threads.h> @@ -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 <atomic> #include <stdint.h> #include <sys/types.h> #include <unordered_map> @@ -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. |