libhidlbase: avoid dlclose

HIDL passthrough has a fatal design flaw! It's not known which
implementation library serves which instance name.

dlclose is extremely problematic:
- some symbols (e.g. like [[clang::no_destroy]] or equiv) may be leaked.
- binder, given multithreaded environment, might have other threads
  using this data
- binder, may send ownership of some of this data out of process

So, proactively removing dlclose here.

As a side effect, this may mean we are keeping extra libraries open when
they were closed before. Though, note:
a. ephemeral dlopen/dlclose is a mostly silent cost which is now visible
b. dlopen/dlclose will definitely be leaking memory
c. for passthrough HALs used by system, only one instance is supported,
   so we shouldn't have this problem
d. passthrough HALs are used as implementation detail for vendors, but:
   i. (a)/(b) still apply here
   ii. most vendor HALs are direct implemenations now (not passthrough).

If this turns out to take more memory on some devices, we can prefer to
improve HIDL's passthrough infrastructure to know which library it needs
to open for which implementation. This would improve device performance
over the existing baseline.

Fixes: 180501607
Test: N/A
Change-Id: I8555ce46b497be4df9d02490bbbe78dbf3255d32

1 files changed, 16 insertions, 7 deletions

diff --git a/transport/ServiceManagement.cpp b/transport/ServiceManagement.cpp
index b51c600..c4e03c3 100644
--- a/transport/ServiceManagement.cpp
+++ b/transport/ServiceManagement.cpp
@@ -417,18 +417,27 @@ struct PassthroughServiceManager : IServiceManager1_1 {
             *(void **)(&generator) = dlsym(handle, sym.c_str());
             if(!generator) {
                 const char* error = dlerror();
-                LOG(ERROR) << "Passthrough lookup opened " << lib
-                           << " but could not find symbol " << sym << ": "
-                           << (error == nullptr ? "unknown error" : error);
-                dlclose(handle);
-                return true;
+                LOG(ERROR) << "Passthrough lookup opened " << lib << " but could not find symbol "
+                           << sym << ": " << (error == nullptr ? "unknown error" : error)
+                           << ". Keeping library open.";
+
+                // dlclose too problematic in multi-threaded environment
+                // dlclose(handle);
+
+                return true;  // continue
             }
 
             ret = (*generator)(name.c_str());
 
             if (ret == nullptr) {
-                dlclose(handle);
-                return true; // this module doesn't provide this instance name
+                LOG(ERROR) << "Could not find instance '" << name.c_str() << "' in library " << lib
+                           << ". Keeping library open.";
+
+                // dlclose too problematic in multi-threaded environment
+                // dlclose(handle);
+
+                // this module doesn't provide this particular instance
+                return true;  // continue
             }
 
             // Actual fqname might be a subclass.