[PATCH] D29955: Allow externally dlopen-ed libraries to be registered as permanent libraries.

[Vassil Vassilev via Phabricator via llvm-commits]

v.g.vassilev added inline comments.


================
Comment at: lib/Support/DynamicLibrary.cpp:52
 
-static DenseSet<void *> *OpenedHandles = nullptr;
+static llvm::ManagedStatic<DenseSet<void *> > OpenedHandles;
 
----------------
vsk wrote:
> This change is a bit out-of-scope for your patch, perhaps? A separate review may be worthwhile to figure out if we really need two layers of locking (one implicit in the ManagedStatic, another with the SymbolsMutex).
Well, this follows the spirit of 'recent' (to us) changes in the area. I'd prefer to keep it together. It seems the ManagedStatic just handles the destruction at shutdown.


================
Comment at: lib/Support/DynamicLibrary.cpp:74
+DynamicLibrary DynamicLibrary::addPermanentLibrary(void *handle) {
+  SmartScopedLock<true> lock(*SymbolsMutex);
 
----------------
vsk wrote:
> Is there a double-lock scenario here? getPermanentLibrary() would lock, then addPermanentLibrary() would lock again?
Yeah, I am not sure how to avoid this...


https://reviews.llvm.org/D29955