lordgamez commented on code in PR #2121:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2121#discussion_r2855687898
##########
libminifi/src/core/extension/ExtensionManager.cpp:
##########
@@ -86,4 +87,14 @@ ExtensionManager::ExtensionManager(const
std::shared_ptr<Configure>& config): lo
}
}
+ExtensionManager::~ExtensionManager() {
+ // Extensions must be destroyed before clearing the registry,
+ // because DLL static destructors (StaticClassType) may still reference
+ // the registry during DLL unload.
+ extensions_.clear();
+ // Clear the class description registry to avoid dangling pointers
+ // to validator objects that lived in the now-unloaded extension DLLs.
+ minifi::ClassDescriptionRegistry::clearClassDescriptions();
Review Comment:
I agree, it's better to reverse these, as I checked that should be okay and
as you said there wouldn't be a time where these dangling pointers are there at
all, updated in
https://github.com/apache/nifi-minifi-cpp/pull/2121/commits/5c93fed7167b30b4f8f245a7814aa38a42463b9a
The map not being synchronized should be okay, as we only populate the map
when initializing the extensions and after that the map is only read and never
modified and only reading it from multiple threads is safe.
##########
libminifi/src/core/extension/ExtensionManager.cpp:
##########
@@ -86,4 +87,14 @@ ExtensionManager::ExtensionManager(const
std::shared_ptr<Configure>& config): lo
}
}
+ExtensionManager::~ExtensionManager() {
+ // Extensions must be destroyed before clearing the registry,
+ // because DLL static destructors (StaticClassType) may still reference
+ // the registry during DLL unload.
+ extensions_.clear();
+ // Clear the class description registry to avoid dangling pointers
+ // to validator objects that lived in the now-unloaded extension DLLs.
+ minifi::ClassDescriptionRegistry::clearClassDescriptions();
Review Comment:
I agree, it's better to reverse these, as I checked that should be okay and
as you said there wouldn't be a time where these dangling pointers are there at
all, updated in
https://github.com/apache/nifi-minifi-cpp/pull/2121/commits/5c93fed7167b30b4f8f245a7814aa38a42463b9a
The map not being synchronized should be okay, as we only populate the map
when initializing the extensions and after that the map is only read and never
modified, and only reading it from multiple threads is safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
