From fc10e8e827d792da075136c9dd2bc0c2b2777929 Mon Sep 17 00:00:00 2001 From: Ruslan Kabatsayev Date: Sat, 9 Dec 2023 23:41:43 +0400 Subject: [PATCH] Fix crash in StelTexture destructor on exit On deletion of StelApp StelTextureMgr is deleted, after which some instances of StelTexture are deleted. But StelTexture destructor logs VRAM usage via StelTextureMgr, accessing a no longer existing object. This commit turns the nonstatic raw pointer to texture manager into a static weak pointer (QPointer), so that it was possible to check whether texture manager still exists before trying to access it. Making it static is supposed to prevent useless copying of this smart pointer while the texture manager is effectively a singleton. Fixes #3427 --- src/core/StelTexture.cpp | 16 ++++++++++++++-- src/core/StelTexture.hpp | 5 +++-- src/core/StelTextureMgr.cpp | 11 ++++++----- src/core/StelTextureMgr.hpp | 2 +- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/core/StelTexture.cpp b/src/core/StelTexture.cpp index 56dc2231c7072..f3b2ae539dfcd 100644 --- a/src/core/StelTexture.cpp +++ b/src/core/StelTexture.cpp @@ -39,13 +39,19 @@ # define GL_TEXTURE_MAX_ANISOTROPY 0x84FE #endif -StelTexture::StelTexture(StelTextureMgr *mgr) - : textureMgr(mgr) +QPointer StelTexture::textureMgr; + +StelTexture::StelTexture() { } StelTexture::~StelTexture() { + // Don't try to access texture manager or OpenGL context if texture + // manager has been deleted, because at this point we are shutting + // down, and the OpenGL context may have also been deleted. + if (!textureMgr) return; + if (id != 0) { /// FS: make sure the correct GL context is bound! @@ -228,6 +234,7 @@ template void StelTexture::startAsyncLoader(T (*functionPointer)(Params...), Args&&...args) { Q_ASSERT(loader==Q_NULLPTR); + if (!textureMgr) return; //own thread pool supported with Qt 5.4+ loader = new QFuture(QtConcurrent::run(textureMgr->loaderThreadPool, functionPointer, std::forward(args)...)); @@ -379,6 +386,11 @@ QByteArray StelTexture::convertToGLFormat(QImage image, GLint& format, GLint& ty bool StelTexture::glLoad(const GLData& data) { + // Don't try to access texture manager or OpenGL context if texture + // manager has been deleted, because at this point we are shutting + // down, and the OpenGL context may have also been deleted. + if (!textureMgr) return false; + if (data.data.isEmpty()) { reportError(data.loaderError.isEmpty() ? "Unknown error" : data.loaderError); diff --git a/src/core/StelTexture.hpp b/src/core/StelTexture.hpp index ceb34263017b6..e24b9531ff30d 100644 --- a/src/core/StelTexture.hpp +++ b/src/core/StelTexture.hpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -143,7 +144,7 @@ private slots: static GLData loadFromData( const QByteArray &data, const int decimateBy); //! Private constructor - StelTexture(StelTextureMgr* mgr); + StelTexture(); //! Wrap an existing GL texture with this object void wrapGLTexture(GLuint texId); @@ -173,7 +174,7 @@ private slots: void startAsyncLoader(T (*functionPointer)(Params...), Args&&...args); //! The parent texture manager - StelTextureMgr* textureMgr = nullptr; + static QPointer textureMgr; QOpenGLFunctions* gl = nullptr; StelTextureParams loadParams; diff --git a/src/core/StelTextureMgr.cpp b/src/core/StelTextureMgr.cpp index f16f2729fa628..b57a4f2be5487 100644 --- a/src/core/StelTextureMgr.cpp +++ b/src/core/StelTextureMgr.cpp @@ -46,6 +46,7 @@ StelTextureMgr::StelTextureMgr(QObject *parent) ctx->functions()->glGetIntegerv(GL_MAX_TEXTURE_SIZE, &maxTexSize); if (maxTexSize<8192) qDebug() << "Max texture size:" << maxTexSize; + StelTexture::textureMgr = this; } StelTextureSP StelTextureMgr::createTexture(const QString& afilename, const StelTexture::StelTextureParams& params) @@ -67,7 +68,7 @@ StelTextureSP StelTextureMgr::createTexture(const QString& afilename, const Stel StelTextureSP cache = lookupCache(canPath); if(!cache.isNull()) return cache; - StelTextureSP tex = StelTextureSP(new StelTexture(this)); + StelTextureSP tex = StelTextureSP(new StelTexture); tex->fullPath = canPath; QImage image(tex->fullPath); @@ -159,7 +160,7 @@ StelTextureSP StelTextureMgr::createTextureThread(const QString& url, const Stel StelTextureSP cache = lookupCache(canPath); if(!cache.isNull()) return cache; - StelTextureSP tex = StelTextureSP(new StelTexture(this)); + StelTextureSP tex = StelTextureSP(new StelTexture); tex->loadParams = params; tex->fullPath = canPath; if (!lazyLoading) @@ -175,7 +176,7 @@ StelTextureSP StelTextureMgr::createTextureThread(const QString& url, const Stel //! Create a texture from a QImage. StelTextureSP StelTextureMgr::createTexture(const QImage &image, const StelTexture::StelTextureParams& params) { - StelTextureSP tex = StelTextureSP(new StelTexture(this)); + StelTextureSP tex = StelTextureSP(new StelTexture); tex->loadParams = params; bool r = tex->glLoad(image); Q_ASSERT(r); @@ -202,7 +203,7 @@ StelTextureSP StelTextureMgr::wrapperForGLTexture(GLuint texId) //no existing tex with this ID found, create a new wrapper - StelTextureSP newTex(new StelTexture(this)); + StelTextureSP newTex(new StelTexture); newTex->wrapGLTexture(texId); if(!newTex->errorOccured) { @@ -230,7 +231,7 @@ StelTextureSP StelTextureMgr::getDitheringTexture(const int samplerToBindTo) const auto texId = ForTextureMgr::makeDitherPatternTexture(gl); - ditheringTexture.reset(new StelTexture(this)); + ditheringTexture.reset(new StelTexture); ditheringTexture->wrapGLTexture(texId); if(!ditheringTexture->errorOccured) { diff --git a/src/core/StelTextureMgr.hpp b/src/core/StelTextureMgr.hpp index e623f467841a3..44bab49e0b189 100644 --- a/src/core/StelTextureMgr.hpp +++ b/src/core/StelTextureMgr.hpp @@ -34,7 +34,7 @@ class QThreadPool; //! @class StelTextureMgr //! Manage textures loading. //! It provides method for loading images in a separate thread. -class StelTextureMgr : QObject +class StelTextureMgr : public QObject { Q_OBJECT public: