Try to handle fonts orphaned from cache gracefully

ImplFontCache::Invalidate deletes unused entries (with zero ref
count), and keeps other entries, but clears everything (including
still used fonts) from its instance list. In the same time, those
fonts' mpFontCache pointers kept pointing to this cache object.
External clients released font instance by calling its cache's
Release method; this itself allows for broken invariants that
cache's mnRef0Count is equal to number of unused font instances
in its list. Also, those fonts never got released, leaking because
ImplFontCache only ever deletes objects in its list.

What is worse, sometimes font caches get deleted after invalidation
(see OutputDevice::ImplClearFontData). As the instance list of the
cache is empty at the point of delete, the cache destructor doesn't
delete those fonts that were orphaned at the moment of invalidation
(those fonts are still used by some client objects, so deleting
them is clearly wrong). But since the font instances still have
cache pointer referring the already deleted cache, releasing the
instances (by calling deleted cache's Release member function)
must lead do some weird results.

This patch moves the Acquire/Release to LogicalFontInstance, which
now checks if its cache pointer is valid, and if it is, the cache
is used to do the work (as before); otherwise, the font handles
its lifetime itself, and deletes itself when its reference counter
is zero. The cache invalidation clears the cache pointer of the
still-used instances.

Change-Id: I29811272dda814cbc81f14668d63e385ce772332
Reviewed-on: https://gerrit.libreoffice.org/47111
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 39 insertions, 4 deletions

diff --git a/vcl/source/font/fontcache.cxx b/vcl/source/font/fontcache.cxx
index 42db74f434d8..874fd1e0cddc 100644
--- a/vcl/source/font/fontcache.cxx
+++ b/vcl/source/font/fontcache.cxx
@@ -168,7 +168,7 @@ LogicalFontInstance* ImplFontCache::GetFontInstance( PhysicalFontCollection cons
     if( pFontInstance ) // cache hit => use existing font instance
     {
         // increase the font instance's reference count
-        Acquire(pFontInstance);
+        pFontInstance->Acquire();
     }
 
     if (!pFontInstance && pFontData)// still no cache hit => create a new font instance
@@ -243,6 +243,7 @@ LogicalFontInstance* ImplFontCache::GetGlyphFallbackFont( PhysicalFontCollection
 void ImplFontCache::Acquire(LogicalFontInstance* pFontInstance)
 {
     assert(pFontInstance->mpFontCache == this);
+    assert(IsFontInList(pFontInstance) && "ImplFontCache::Acquire() - font absent in the cache");
 
     if (0 == pFontInstance->mnRefCount++)
         --mnRef0Count;
@@ -252,6 +253,8 @@ void ImplFontCache::Release(LogicalFontInstance* pFontInstance)
 {
     static const int FONTCACHE_MAX = getenv("LO_TESTNAME") ? 1 : 50;
 
+    assert(pFontInstance->mpFontCache == this);
+    assert(IsFontInList(pFontInstance) && "ImplFontCache::Release() - font absent in the cache");
     assert(pFontInstance->mnRefCount > 0 && "ImplFontCache::Release() - font refcount underflow");
     if( --pFontInstance->mnRefCount > 0 )
         return;
@@ -282,6 +285,12 @@ void ImplFontCache::Release(LogicalFontInstance* pFontInstance)
     assert(mnRef0Count==0 && "ImplFontCache::Release() - refcount0 mismatch");
 }
 
+bool ImplFontCache::IsFontInList(const LogicalFontInstance* pFont) const
+{
+    auto Pred = [pFont](const FontInstanceList::value_type& el) -> bool { return el.second == pFont; };
+    return std::find_if(maFontInstanceList.begin(), maFontInstanceList.end(), Pred) != maFontInstanceList.end();
+}
+
 int ImplFontCache::CountUnreferencedEntries() const
 {
     size_t nCount = 0;
@@ -307,7 +316,12 @@ void ImplFontCache::Invalidate()
     {
         LogicalFontInstance* pFontEntry = (*it).second;
         if( pFontEntry->mnRefCount > 0 )
+        {
+            // These fonts will become orphans after clearing the list below;
+            // allow them to control their life from now on and wish good luck :)
+            pFontEntry->mpFontCache = nullptr;
             continue;
+        }
 
         delete pFontEntry;
         --mnRef0Count;
diff --git a/vcl/source/font/fontinstance.cxx b/vcl/source/font/fontinstance.cxx
index 6d566264bffc..05263e863aef 100644
--- a/vcl/source/font/fontinstance.cxx
+++ b/vcl/source/font/fontinstance.cxx
@@ -38,16 +38,16 @@ namespace std
 
 
 LogicalFontInstance::LogicalFontInstance( const FontSelectPattern& rFontSelData )
-    : mpFontCache(nullptr)
-    , maFontSelData( rFontSelData )
+    : maFontSelData( rFontSelData )
     , mxFontMetric( new ImplFontMetricData( rFontSelData ))
     , mpConversion( nullptr )
     , mnLineHeight( 0 )
-    , mnRefCount( 1 )
     , mnOwnOrientation( 0 )
     , mnOrientation( 0 )
     , mbInit( false )
     , mpUnicodeFallbackList( nullptr )
+    , mpFontCache( nullptr )
+    , mnRefCount( 1 )
 {
     maFontSelData.mpFontInstance = this;
 }
@@ -59,6 +59,27 @@ LogicalFontInstance::~LogicalFontInstance()
     mxFontMetric = nullptr;
 }
 
+void LogicalFontInstance::Acquire()
+{
+    assert(mnRefCount < std::numeric_limits<decltype(mnRefCount)>::max()
+        && "LogicalFontInstance::Release() - refcount overflow");
+    if (mpFontCache)
+        mpFontCache->Acquire(this);
+    else
+        ++mnRefCount;
+}
+
+void LogicalFontInstance::Release()
+{
+    assert(mnRefCount > 0 && "LogicalFontInstance::Release() - refcount underflow");
+
+    if (mpFontCache)
+        mpFontCache->Release(this);
+    else
+        if (--mnRefCount == 0)
+            delete this;
+}
+
 void LogicalFontInstance::AddFallbackForUnicode( sal_UCS4 cChar, FontWeight eWeight, const OUString& rFontName )
 {
     if( !mpUnicodeFallbackList )