From aa0e3365448565e560ac9a6890a60f50b31a3cf9 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Fri, 9 Aug 2024 22:34:59 +0000 Subject: [PATCH 01/11] workaround crash by not unloading RenderSystem_GL Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 83 ++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 4 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index d201ee1bf..289d8f668 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -38,6 +38,9 @@ typedef khronos_intptr_t GLintptr; # include +#include +#include + #include #include @@ -66,6 +69,61 @@ class gz::rendering::OgreRenderEnginePrivate using namespace gz; using namespace rendering; +// Plugin loading and unloading code adapted from Ogre. +// Unloading the RenderSystem_GL plugin was found to cause a crash when the +// rendering thread exits. This only happens on Ubuntu 24.04 when running +// ogre using system debs, see +// https://github.com/gazebosim/gz-rendering/issues/1007 +// To workaournd ths issue, we adapth the code for loadng and unloading +// ogre plugin here. We load the RenderSystem_GL library manually and store it +// in gz-rendering and on shutdown we stop the plugin and delete the lib without +// unloading it. +// \todo(iche033) Find a proper way to unload the library without causing a +// crash. + +typedef void (*DLL_START_PLUGIN)(void); +typedef void (*DLL_STOP_PLUGIN)(void); +std::vector mPluginLibs; + +void loadPlugin(const std::string &_pluginName) +{ + // Load plugin library + Ogre::DynLib *lib = OGRE_NEW Ogre::DynLib(_pluginName); + lib->load(); + if (std::find(mPluginLibs.begin(), mPluginLibs.end(), lib) == + mPluginLibs.end()) + { + mPluginLibs.push_back(lib); + // Call startup function + #ifdef __GNUC__ + __extension__ + #endif + DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)lib->getSymbol("dllStartPlugin"); + if (!pFunc) + OGRE_EXCEPT(Ogre::Exception::ERR_ITEM_NOT_FOUND, + "Cannot find symbol dllStartPlugin in library " + _pluginName, + "Ogre::Root::loadPlugin"); + pFunc(); + } +} + +void unloadPlugin(Ogre::DynLib *_pluginLib) +{ + // Call plugin shutdown + #ifdef __GNUC__ + __extension__ + #endif + DLL_STOP_PLUGIN pFunc = reinterpret_cast( + _pluginLib->getSymbol("dllStopPlugin")); + pFunc(); + + // Unload library & destroy + // \todo(iche033) Unloading the RenderSystem_GL system causes a crash + // on thread exit so commented out for now. + // _pluginLib->unload(); + delete _pluginLib; +} + ////////////////////////////////////////////////// OgreRenderEnginePlugin::OgreRenderEnginePlugin() { @@ -121,6 +179,14 @@ void OgreRenderEngine::Destroy() try { // TODO(anyone): do we need to catch segfault on delete? + + // manually shutdown render system and stop the RenderSystem_GL library + this->ogreRoot->shutdown(); + this->ogreRoot->getRenderSystem()->shutdown(); + this->ogreRoot->setRenderSystem(nullptr); + for (auto &lib : mPluginLibs) + unloadPlugin(lib); + delete this->ogreRoot; } catch (...) @@ -458,17 +524,26 @@ void OgreRenderEngine::LoadPlugins() for (piter = plugins.begin(); piter != plugins.end(); ++piter) { + + bool isGLPlugin = std::string(*piter).find("RenderSystem_GL") + != std::string::npos; try { - // Load the plugin into OGRE - this->ogreRoot->loadPlugin(*piter+extension); + // Load the plugin + if (isGLPlugin) + loadPlugin(*piter+extension); + else + this->ogreRoot->loadPlugin(*piter+extension); } catch(Ogre::Exception &e) { try { - // Load the debug plugin into OGRE - this->ogreRoot->loadPlugin(*piter+"_d"+extension); + // Load the debug plugin + if (isGLPlugin) + loadPlugin(*piter+"_d"+extension); + else + this->ogreRoot->loadPlugin(*piter+"_d"+extension); } catch(Ogre::Exception &ed) { From acc22c404a6e15547e3971c848e92e5a3abf652f Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Sat, 10 Aug 2024 01:19:05 +0000 Subject: [PATCH 02/11] fix crashes Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index 289d8f668..105a41b5c 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -180,13 +180,20 @@ void OgreRenderEngine::Destroy() { // TODO(anyone): do we need to catch segfault on delete? - // manually shutdown render system and stop the RenderSystem_GL library + // Manually shutdown ogre and stop the RenderSystem_GL library + // without unloading it. + // \todo(iche033) replace this whole block with: + // delete this->ogreRoot + // once we are able to unload the RenderSystem_GL library without + // crashing. this->ogreRoot->shutdown(); - this->ogreRoot->getRenderSystem()->shutdown(); + this->ogreRoot->destroySceneManager( + this->ogreRoot->_getCurrentSceneManager()); + Ogre::TextureManager::getSingletonPtr()->unloadAll(); + Ogre::ShadowTextureManager::getSingletonPtr()->clear(); this->ogreRoot->setRenderSystem(nullptr); for (auto &lib : mPluginLibs) unloadPlugin(lib); - delete this->ogreRoot; } catch (...) From 83c5624368e17354ac2adc39b6f8397574d6bf87 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Sat, 10 Aug 2024 04:14:05 +0000 Subject: [PATCH 03/11] unload plugin at the right time Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 47 +++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index 105a41b5c..a33b8e4f6 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -40,6 +40,7 @@ typedef khronos_intptr_t GLintptr; #include #include +#include #include @@ -80,7 +81,6 @@ using namespace rendering; // unloading it. // \todo(iche033) Find a proper way to unload the library without causing a // crash. - typedef void (*DLL_START_PLUGIN)(void); typedef void (*DLL_STOP_PLUGIN)(void); std::vector mPluginLibs; @@ -124,6 +124,30 @@ void unloadPlugin(Ogre::DynLib *_pluginLib) delete _pluginLib; } +/// A dummy plugin class that is installed to the ogre root before shutdown. +/// We use this as a hook to get a callback from ogre root when all plugins +/// are being unloaded. This is so that we can unload the RenderSystem_GL +/// at the right time. +class DummyPlugin : public Ogre::Plugin +{ + public: DummyPlugin() {} + public: const Ogre::String& getName() const + { + return pluginName; + } + public: void install() {} + public: void initialise() {} + public: void shutdown() {} + public: void uninstall() + { + for (auto &lib : mPluginLibs) + unloadPlugin(lib); + mPluginLibs.clear(); + } + const Ogre::String pluginName = "dummy"; +}; +DummyPlugin gDummyPlugin; + ////////////////////////////////////////////////// OgreRenderEnginePlugin::OgreRenderEnginePlugin() { @@ -176,24 +200,13 @@ void OgreRenderEngine::Destroy() if (ogreRoot) { + // TODO(anyone): do we need to catch segfault on delete? try { - // TODO(anyone): do we need to catch segfault on delete? - - // Manually shutdown ogre and stop the RenderSystem_GL library - // without unloading it. - // \todo(iche033) replace this whole block with: - // delete this->ogreRoot - // once we are able to unload the RenderSystem_GL library without - // crashing. - this->ogreRoot->shutdown(); - this->ogreRoot->destroySceneManager( - this->ogreRoot->_getCurrentSceneManager()); - Ogre::TextureManager::getSingletonPtr()->unloadAll(); - Ogre::ShadowTextureManager::getSingletonPtr()->clear(); - this->ogreRoot->setRenderSystem(nullptr); - for (auto &lib : mPluginLibs) - unloadPlugin(lib); + // Install a dummy plugin that allows use to manually shutdown ogre + // and stop the RenderSystem_GL library. + this->ogreRoot->installPlugin(&gDummyPlugin); + delete this->ogreRoot; } catch (...) From 291032610c2669c7d782c88f155b605b8cd1fd3b Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Wed, 14 Aug 2024 22:54:59 +0000 Subject: [PATCH 04/11] simplify code Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 88 ++++++------------------------------ 1 file changed, 14 insertions(+), 74 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index a33b8e4f6..3ce801a25 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -70,83 +70,27 @@ class gz::rendering::OgreRenderEnginePrivate using namespace gz; using namespace rendering; -// Plugin loading and unloading code adapted from Ogre. // Unloading the RenderSystem_GL plugin was found to cause a crash when the // rendering thread exits. This only happens on Ubuntu 24.04 when running // ogre using system debs, see // https://github.com/gazebosim/gz-rendering/issues/1007 -// To workaournd ths issue, we adapth the code for loadng and unloading -// ogre plugin here. We load the RenderSystem_GL library manually and store it -// in gz-rendering and on shutdown we stop the plugin and delete the lib without -// unloading it. +// To workaround ths issue, we adapt the code for loading the ogre plugin here. +// We load the RenderSystem_GL library manually and store it in gz-rendering. +// On shutdown, ogre root will uninstall this plugin and delete the GL +// render system objects. However, gz-rendering does not unload this plugin. // \todo(iche033) Find a proper way to unload the library without causing a // crash. typedef void (*DLL_START_PLUGIN)(void); -typedef void (*DLL_STOP_PLUGIN)(void); -std::vector mPluginLibs; - -void loadPlugin(const std::string &_pluginName) -{ - // Load plugin library - Ogre::DynLib *lib = OGRE_NEW Ogre::DynLib(_pluginName); - lib->load(); - if (std::find(mPluginLibs.begin(), mPluginLibs.end(), lib) == - mPluginLibs.end()) - { - mPluginLibs.push_back(lib); - // Call startup function - #ifdef __GNUC__ - __extension__ - #endif - DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)lib->getSymbol("dllStartPlugin"); - if (!pFunc) - OGRE_EXCEPT(Ogre::Exception::ERR_ITEM_NOT_FOUND, - "Cannot find symbol dllStartPlugin in library " + _pluginName, - "Ogre::Root::loadPlugin"); - pFunc(); - } -} - -void unloadPlugin(Ogre::DynLib *_pluginLib) -{ - // Call plugin shutdown - #ifdef __GNUC__ - __extension__ - #endif - DLL_STOP_PLUGIN pFunc = reinterpret_cast( - _pluginLib->getSymbol("dllStopPlugin")); - pFunc(); - - // Unload library & destroy - // \todo(iche033) Unloading the RenderSystem_GL system causes a crash - // on thread exit so commented out for now. - // _pluginLib->unload(); - delete _pluginLib; -} +void *glPluginHandle; -/// A dummy plugin class that is installed to the ogre root before shutdown. -/// We use this as a hook to get a callback from ogre root when all plugins -/// are being unloaded. This is so that we can unload the RenderSystem_GL -/// at the right time. -class DummyPlugin : public Ogre::Plugin +void loadGLPlugin(const std::string &_pluginName) { - public: DummyPlugin() {} - public: const Ogre::String& getName() const - { - return pluginName; - } - public: void install() {} - public: void initialise() {} - public: void shutdown() {} - public: void uninstall() - { - for (auto &lib : mPluginLibs) - unloadPlugin(lib); - mPluginLibs.clear(); - } - const Ogre::String pluginName = "dummy"; -}; -DummyPlugin gDummyPlugin; + glPluginHandle = dlopen(_pluginName.c_str(), RTLD_LAZY | RTLD_LOCAL); + DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)DYNLIB_GETSYM( + glPluginHandle, "dllStartPlugin"); + pFunc(); + return; +} ////////////////////////////////////////////////// OgreRenderEnginePlugin::OgreRenderEnginePlugin() @@ -203,10 +147,6 @@ void OgreRenderEngine::Destroy() // TODO(anyone): do we need to catch segfault on delete? try { - // Install a dummy plugin that allows use to manually shutdown ogre - // and stop the RenderSystem_GL library. - this->ogreRoot->installPlugin(&gDummyPlugin); - delete this->ogreRoot; } catch (...) @@ -551,7 +491,7 @@ void OgreRenderEngine::LoadPlugins() { // Load the plugin if (isGLPlugin) - loadPlugin(*piter+extension); + loadGLPlugin(*piter+extension); else this->ogreRoot->loadPlugin(*piter+extension); } @@ -561,7 +501,7 @@ void OgreRenderEngine::LoadPlugins() { // Load the debug plugin if (isGLPlugin) - loadPlugin(*piter+"_d"+extension); + loadGLPlugin(*piter+"_d"+extension); else this->ogreRoot->loadPlugin(*piter+"_d"+extension); } From 46690abda13b32d8382f81e4b701e236522cadd5 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Wed, 14 Aug 2024 22:58:03 +0000 Subject: [PATCH 05/11] remove headers Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index 3ce801a25..e516034ba 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -39,8 +39,6 @@ typedef khronos_intptr_t GLintptr; # include #include -#include -#include #include From e037bb1f1abc8c6839e7a8553507c3d969b7e003 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 15 Aug 2024 00:03:37 +0000 Subject: [PATCH 06/11] add include Signed-off-by: Ian Chen --- src/RenderEngineManager.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/RenderEngineManager.cc b/src/RenderEngineManager.cc index 8e3a6f0f4..9f9d4e571 100644 --- a/src/RenderEngineManager.cc +++ b/src/RenderEngineManager.cc @@ -15,6 +15,8 @@ * */ +#include + #include #include From 5256bbdac3617b67f5e740f354d15adb004ca81a Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 15 Aug 2024 00:17:48 +0000 Subject: [PATCH 07/11] fix Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 2 ++ src/RenderEngineManager.cc | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index e516034ba..b24149c1e 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -36,6 +36,8 @@ typedef khronos_intptr_t GLintptr; #include #endif +#include + # include #include diff --git a/src/RenderEngineManager.cc b/src/RenderEngineManager.cc index 9f9d4e571..8e3a6f0f4 100644 --- a/src/RenderEngineManager.cc +++ b/src/RenderEngineManager.cc @@ -15,8 +15,6 @@ * */ -#include - #include #include From 71588d6209ed7abe064c6db34318f1be50bc1f90 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 15 Aug 2024 00:37:50 +0000 Subject: [PATCH 08/11] more fixes for windows Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index b24149c1e..eb404dfae 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -81,7 +81,7 @@ using namespace rendering; // \todo(iche033) Find a proper way to unload the library without causing a // crash. typedef void (*DLL_START_PLUGIN)(void); -void *glPluginHandle; +DYNLIB_HANDLE glPluginHandle; void loadGLPlugin(const std::string &_pluginName) { From 63733d03f46c5049ab5cb70202427180058130b0 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 15 Aug 2024 01:27:53 +0000 Subject: [PATCH 09/11] more fixes for windows Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index eb404dfae..b9ab27b29 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -81,12 +81,12 @@ using namespace rendering; // \todo(iche033) Find a proper way to unload the library without causing a // crash. typedef void (*DLL_START_PLUGIN)(void); -DYNLIB_HANDLE glPluginHandle; +void *glPluginHandle; void loadGLPlugin(const std::string &_pluginName) { glPluginHandle = dlopen(_pluginName.c_str(), RTLD_LAZY | RTLD_LOCAL); - DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)DYNLIB_GETSYM( + DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)dlsym( glPluginHandle, "dllStartPlugin"); pFunc(); return; From 26425b8650269cae2c2271980e1e799c91c09137 Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 15 Aug 2024 03:25:06 +0000 Subject: [PATCH 10/11] one more try for windows Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index b9ab27b29..67dd89243 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -81,12 +81,12 @@ using namespace rendering; // \todo(iche033) Find a proper way to unload the library without causing a // crash. typedef void (*DLL_START_PLUGIN)(void); -void *glPluginHandle; +DYNLIB_HANDLE glPluginHandle; void loadGLPlugin(const std::string &_pluginName) { - glPluginHandle = dlopen(_pluginName.c_str(), RTLD_LAZY | RTLD_LOCAL); - DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)dlsym( + glPluginHandle = DYNLIB_LOAD(_pluginName.c_str()); + DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)DYNLIB_GETSYM( glPluginHandle, "dllStartPlugin"); pFunc(); return; From 6c2b0dcc98165a7c47756c218d1dac153a1ff8fd Mon Sep 17 00:00:00 2001 From: Ian Chen Date: Thu, 15 Aug 2024 04:16:46 +0000 Subject: [PATCH 11/11] use ifdef Signed-off-by: Ian Chen --- ogre/src/OgreRenderEngine.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ogre/src/OgreRenderEngine.cc b/ogre/src/OgreRenderEngine.cc index 67dd89243..5d7996d70 100644 --- a/ogre/src/OgreRenderEngine.cc +++ b/ogre/src/OgreRenderEngine.cc @@ -85,7 +85,11 @@ DYNLIB_HANDLE glPluginHandle; void loadGLPlugin(const std::string &_pluginName) { +#ifdef _WIN32 glPluginHandle = DYNLIB_LOAD(_pluginName.c_str()); +#else + glPluginHandle = dlopen(_pluginName.c_str(), RTLD_LAZY | RTLD_LOCAL); +#endif DLL_START_PLUGIN pFunc = (DLL_START_PLUGIN)DYNLIB_GETSYM( glPluginHandle, "dllStartPlugin"); pFunc();