vcl: fix lifecycle errors & memory corruption.

FixedTextureAtlasManager should use ref-counted textures properly.
Also - dispose embedded textures early in VCL shutdown while we have
a valid OpenGLContext.
Also - dispose the native widget control cache earlier too.

Change-Id: Id3f7a1c3b331496616f36cbf02f83737505278a5
Reviewed-on: https://gerrit.libreoffice.org/21148
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>

3 files changed, 33 insertions, 20 deletions

diff --git a/vcl/opengl/FixedTextureAtlas.cxx b/vcl/opengl/FixedTextureAtlas.cxx
index 8a3e927b4698..80c1cfe496c7 100644
--- a/vcl/opengl/FixedTextureAtlas.cxx
+++ b/vcl/opengl/FixedTextureAtlas.cxx
@@ -24,11 +24,21 @@ FixedTextureAtlasManager::FixedTextureAtlasManager(int nWidthFactor, int nHeight
 {
 }
 
+FixedTextureAtlasManager::~FixedTextureAtlasManager()
+{
+    for (auto i = mpTextures.begin(); i != mpTextures.end(); ++i)
+    {
+        // Free texture early in VCL shutdown while we have a context.
+        (*i)->Dispose();
+        (*i)->DecreaseRefCount(0);
+    }
+}
+
 void FixedTextureAtlasManager::CreateNewTexture()
 {
     int nTextureWidth = mWidthFactor  * mSubTextureSize;
     int nTextureHeight = mHeightFactor * mSubTextureSize;
-    mpTextures.push_back(std::unique_ptr<ImplOpenGLTexture>(new ImplOpenGLTexture(nTextureWidth, nTextureHeight, true)));
+    mpTextures.push_back(new ImplOpenGLTexture(nTextureWidth, nTextureHeight, true));
     mpTextures.back()->InitializeSlots(mWidthFactor * mHeightFactor);
 }
 
@@ -36,21 +46,21 @@ OpenGLTexture FixedTextureAtlasManager::InsertBuffer(int nWidth, int nHeight, in
 {
     ImplOpenGLTexture* pTexture = nullptr;
 
-    auto funFreeSlot = [] (std::unique_ptr<ImplOpenGLTexture>& mpTexture)
+    auto funFreeSlot = [] (ImplOpenGLTexture *mpTexture)
     {
         return mpTexture->mnFreeSlots > 0;
     };
 
-    auto aIterator = std::find_if(mpTextures.begin(), mpTextures.end(), funFreeSlot);
+    auto it = std::find_if(mpTextures.begin(), mpTextures.end(), funFreeSlot);
 
-    if (aIterator != mpTextures.end())
+    if (it != mpTextures.end())
     {
-        pTexture = (*aIterator).get();
+        pTexture = *it;
     }
     else
     {
         CreateNewTexture();
-        pTexture = mpTextures.back().get();
+        pTexture = mpTextures.back();
     }
 
     int nSlot = pTexture->FindFreeSlot();
diff --git a/vcl/opengl/texture.cxx b/vcl/opengl/texture.cxx
index 303b8b8f19d6..3b484c00e6e2 100644
--- a/vcl/opengl/texture.cxx
+++ b/vcl/opengl/texture.cxx
@@ -157,6 +157,11 @@ GLuint ImplOpenGLTexture::AddStencil()
 ImplOpenGLTexture::~ImplOpenGLTexture()
 {
     VCL_GL_INFO( "~OpenGLTexture " << mnTexture );
+    Dispose();
+}
+
+void ImplOpenGLTexture::Dispose()
+{
     if( mnTexture != 0 )
     {
         OpenGLVCLContextZone aContextZone;
@@ -173,8 +178,12 @@ ImplOpenGLTexture::~ImplOpenGLTexture()
         }
 
         if( mnOptStencil != 0 )
+        {
             glDeleteRenderbuffers( 1, &mnOptStencil );
+            mnOptStencil = 0;
+        }
         glDeleteTextures( 1, &mnTexture );
+        mnTexture = 0;
     }
 }
 
@@ -285,16 +294,12 @@ OpenGLTexture::OpenGLTexture( const OpenGLTexture& rTexture,
 OpenGLTexture::~OpenGLTexture()
 {
     if (mpImpl)
-    {
         mpImpl->DecreaseRefCount(mnSlotNumber);
-        if (!mpImpl->ExistRefs())
-            delete mpImpl;
-    }
 }
 
 bool OpenGLTexture::IsUnique() const
 {
-    return ( mpImpl == nullptr || mpImpl->mnRefCount == 1 );
+    return mpImpl == nullptr || mpImpl->IsUnique();
 }
 
 GLuint OpenGLTexture::Id() const
@@ -484,11 +489,7 @@ OpenGLTexture&  OpenGLTexture::operator=( const OpenGLTexture& rTexture )
     }
 
     if (mpImpl)
-    {
         mpImpl->DecreaseRefCount(mnSlotNumber);
-        if (!mpImpl->ExistRefs())
-            delete mpImpl;
-    }
 
     maRect = rTexture.maRect;
     mpImpl = rTexture.mpImpl;
diff --git a/vcl/opengl/x11/gdiimpl.cxx b/vcl/opengl/x11/gdiimpl.cxx
index effc81b3c815..b1bc724c86ee 100644
--- a/vcl/opengl/x11/gdiimpl.cxx
+++ b/vcl/opengl/x11/gdiimpl.cxx
@@ -8,6 +8,7 @@
  */
 
 #include <vcl/salbtype.hxx>
+#include <vcl/lazydelete.hxx>
 
 #include <svdata.hxx>
 
@@ -105,7 +106,7 @@ bool X11OpenGLSalGraphicsImpl::FillPixmapFromScreen( X11Pixmap* pPixmap, int nX,
 typedef typename std::pair<ControlCacheKey, std::unique_ptr<TextureCombo>> ControlCachePair;
 typedef o3tl::lru_map<ControlCacheKey, std::unique_ptr<TextureCombo>, ControlCacheHashFunction> ControlCacheType;
 
-ControlCacheType gTextureCache(200);
+vcl::DeleteOnDeinit<ControlCacheType> gTextureCache(new ControlCacheType(200));
 
 bool X11OpenGLSalGraphicsImpl::RenderPixmap(X11Pixmap* pPixmap, X11Pixmap* pMask, int nX, int nY, TextureCombo& rCombo)
 {
@@ -190,12 +191,12 @@ bool X11OpenGLSalGraphicsImpl::TryRenderCachedNativeControl(ControlCacheKey& rCo
 {
     static bool gbCacheEnabled = !getenv("SAL_WITHOUT_WIDGET_CACHE");
 
-    if (!gbCacheEnabled)
+    if (!gbCacheEnabled || !gTextureCache.get())
         return false;
 
-    ControlCacheType::const_iterator iterator = gTextureCache.find(rControlCacheKey);
+    ControlCacheType::const_iterator iterator = gTextureCache.get()->find(rControlCacheKey);
 
-    if (iterator == gTextureCache.end())
+    if (iterator == gTextureCache.get()->end())
         return false;
 
     const std::unique_ptr<TextureCombo>& pCombo = iterator->second;
@@ -229,7 +230,8 @@ bool X11OpenGLSalGraphicsImpl::RenderAndCacheNativeControl(X11Pixmap* pPixmap, X
         return true;
 
     ControlCachePair pair(aControlCacheKey, std::move(pCombo));
-    gTextureCache.insert(std::move(pair));
+    if (gTextureCache.get())
+        gTextureCache.get()->insert(std::move(pair));
 
     return bResult;
 }