[Libreoffice-commits] core.git: vcl/inc vcl/source vcl/win
Mike Kaganski
mike.kaganski at collabora.com
Thu Dec 28 12:04:53 UTC 2017
vcl/inc/fontinstance.hxx | 8 ++++++--
vcl/inc/impfontcache.hxx | 13 ++++++++-----
vcl/source/font/fontcache.cxx | 16 +++++++++++++++-
vcl/source/font/fontinstance.cxx | 27 ++++++++++++++++++++++++---
vcl/source/gdi/print.cxx | 8 ++++----
vcl/source/gdi/virdev.cxx | 2 +-
vcl/source/outdev/font.cxx | 10 +++++-----
vcl/source/outdev/outdev.cxx | 2 +-
vcl/source/outdev/outdevstate.cxx | 2 +-
vcl/source/window/window.cxx | 2 +-
vcl/win/gdi/salfont.cxx | 6 +++---
11 files changed, 69 insertions(+), 27 deletions(-)
New commits:
commit b0203670492d5af7f963e66ef702f36c87b6b694
Author: Mike Kaganski <mike.kaganski at collabora.com>
Date: Thu Dec 28 00:35:23 2017 +0300
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 at collabora.com>
Tested-by: Mike Kaganski <mike.kaganski at collabora.com>
diff --git a/vcl/inc/fontinstance.hxx b/vcl/inc/fontinstance.hxx
index 7595c79036cd..be6df2c0fbad 100644
--- a/vcl/inc/fontinstance.hxx
+++ b/vcl/inc/fontinstance.hxx
@@ -36,17 +36,16 @@ class VCL_PLUGIN_PUBLIC LogicalFontInstance
// just declaring the factory function doesn't work AKA
// friend LogicalFontInstance* PhysicalFontFace::CreateFontInstance(const FontSelectPattern&) const;
friend class PhysicalFontFace;
+ friend class ImplFontCache;
public: // TODO: make data members private
virtual ~LogicalFontInstance();
- ImplFontCache * mpFontCache;
FontSelectPattern maFontSelData; // FontSelectionData
ImplFontMetricDataRef mxFontMetric; // Font attributes
const ConvertChar* mpConversion; // used e.g. for StarBats->StarSymbol
long mnLineHeight;
- sal_uInt32 mnRefCount;
short mnOwnOrientation; // text angle if lower layers don't rotate text themselves
short mnOrientation; // text angle in 3600 system
bool mbInit; // true if maFontMetric member is valid
@@ -55,6 +54,9 @@ public: // TODO: make data members private
bool GetFallbackForUnicode( sal_UCS4, FontWeight eWeight, OUString* pFontName ) const;
void IgnoreFallbackForUnicode( sal_UCS4, FontWeight eWeight, const OUString& rFontName );
+ void Acquire();
+ void Release();
+
protected:
explicit LogicalFontInstance(const FontSelectPattern&);
@@ -64,6 +66,8 @@ private:
// TODO: at least the ones which just differ in orientation, stretching or height
typedef ::std::unordered_map< ::std::pair<sal_UCS4,FontWeight>, OUString > UnicodeFallbackList;
UnicodeFallbackList* mpUnicodeFallbackList;
+ ImplFontCache * mpFontCache;
+ sal_uInt32 mnRefCount;
};
#endif // INCLUDED_VCL_INC_FONTINSTANCE_HXX
diff --git a/vcl/inc/impfontcache.hxx b/vcl/inc/impfontcache.hxx
index eb00009028b1..c5c7a4718d19 100644
--- a/vcl/inc/impfontcache.hxx
+++ b/vcl/inc/impfontcache.hxx
@@ -33,6 +33,8 @@ class PhysicalFontCollection;
class ImplFontCache
{
+ // For access to Acquire and Release
+ friend class LogicalFontInstance;
private:
LogicalFontInstance* mpFirstEntry;
int mnRef0Count; // number of unreferenced LogicalFontInstances
@@ -44,6 +46,12 @@ private:
FontInstanceList maFontInstanceList;
int CountUnreferencedEntries() const;
+ bool IsFontInList(const LogicalFontInstance* pFont) const;
+
+ /// Increase the refcount of the given LogicalFontInstance.
+ void Acquire(LogicalFontInstance*);
+ /// Decrease the refcount and potentially cleanup the entries with zero refcount from the cache.
+ void Release(LogicalFontInstance*);
public:
ImplFontCache();
@@ -55,11 +63,6 @@ public:
LogicalFontInstance* GetGlyphFallbackFont( PhysicalFontCollection const *, FontSelectPattern&,
int nFallbackLevel, OUString& rMissingCodes );
- /// Increase the refcount of the given LogicalFontInstance.
- void Acquire(LogicalFontInstance*);
- /// Decrease the refcount and potentially cleanup the entries with zero refcount from the cache.
- void Release(LogicalFontInstance*);
-
void Invalidate();
};
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 )
diff --git a/vcl/source/gdi/print.cxx b/vcl/source/gdi/print.cxx
index db9eafd1546d..d5d472be2eeb 100644
--- a/vcl/source/gdi/print.cxx
+++ b/vcl/source/gdi/print.cxx
@@ -593,7 +593,7 @@ void Printer::ImplReleaseFonts()
if ( mpFontInstance )
{
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
mpFontInstance = nullptr;
}
@@ -970,7 +970,7 @@ void Printer::dispose()
// TODO: consolidate duplicate cleanup by Printer and OutputDevice
if ( mpFontInstance )
{
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
mpFontInstance = nullptr;
}
if ( mpDeviceFontList )
@@ -1118,7 +1118,7 @@ bool Printer::SetPrinterProps( const Printer* pPrinter )
pSVData->mpDefInst->DestroyInfoPrinter( mpInfoPrinter );
if ( mpFontInstance )
{
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
mpFontInstance = nullptr;
}
if ( mpDeviceFontList )
@@ -1161,7 +1161,7 @@ bool Printer::SetPrinterProps( const Printer* pPrinter )
if ( mpFontInstance )
{
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
mpFontInstance = nullptr;
}
if ( mpDeviceFontList )
diff --git a/vcl/source/gdi/virdev.cxx b/vcl/source/gdi/virdev.cxx
index f998f012fe4b..a867494400eb 100644
--- a/vcl/source/gdi/virdev.cxx
+++ b/vcl/source/gdi/virdev.cxx
@@ -495,7 +495,7 @@ void VirtualDevice::ImplSetReferenceDevice( RefDevMode i_eRefDevMode, sal_Int32
// => clean up the original font lists before getting new ones
if ( mpFontInstance )
{
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
mpFontInstance = nullptr;
}
if ( mpDeviceFontList )
diff --git a/vcl/source/outdev/font.cxx b/vcl/source/outdev/font.cxx
index 778f6e2ed1a7..077159bebe83 100644
--- a/vcl/source/outdev/font.cxx
+++ b/vcl/source/outdev/font.cxx
@@ -479,7 +479,7 @@ void OutputDevice::ImplClearFontData( const bool bNewFontLists )
// the currently selected logical font is no longer needed
if ( mpFontInstance )
{
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
mpFontInstance = nullptr;
}
@@ -900,7 +900,7 @@ vcl::Font OutputDevice::GetDefaultFont( DefaultFontType nType, LanguageType eLan
aFont.SetFamilyName( pFontInstance->maFontSelData.mpFontData->GetFamilyName() );
else
aFont.SetFamilyName( pFontInstance->maFontSelData.maTargetName );
- pOutDev->mpFontCache->Release(pFontInstance);
+ pFontInstance->Release();
}
}
}
@@ -1047,7 +1047,7 @@ bool OutputDevice::ImplNewFont() const
LogicalFontInstance* pOldFontInstance = mpFontInstance;
mpFontInstance = mpFontCache->GetFontInstance( mpFontCollection, maFont, aSize, fExactHeight );
if( pOldFontInstance )
- mpFontCache->Release( pOldFontInstance );
+ pOldFontInstance->Release();
LogicalFontInstance* pFontInstance = mpFontInstance;
@@ -1375,7 +1375,7 @@ std::unique_ptr<SalLayout> OutputDevice::ImplGlyphFallbackLayout( std::unique_pt
if( mpFontInstance->maFontSelData.mpFontData == aFontSelData.mpFontData &&
aMissingCodes.indexOf(0x202F) == -1 )
{
- mpFontCache->Release( pFallbackFont );
+ pFallbackFont->Release();
continue;
}
}
@@ -1393,7 +1393,7 @@ std::unique_ptr<SalLayout> OutputDevice::ImplGlyphFallbackLayout( std::unique_pt
pMultiSalLayout->SetIncomplete(true);
}
- mpFontCache->Release( pFallbackFont );
+ pFallbackFont->Release();
// break when this fallback was sufficient
if( !rLayoutArgs.PrepareFallback() )
diff --git a/vcl/source/outdev/outdev.cxx b/vcl/source/outdev/outdev.cxx
index 84408cd9eefe..5066c3b87071 100644
--- a/vcl/source/outdev/outdev.cxx
+++ b/vcl/source/outdev/outdev.cxx
@@ -175,7 +175,7 @@ void OutputDevice::dispose()
// release the active font instance
if( mpFontInstance )
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
// remove cached results of GetDevFontList/GetDevSizeList
// TODO: use smart pointers for them
diff --git a/vcl/source/outdev/outdevstate.cxx b/vcl/source/outdev/outdevstate.cxx
index a7f626a4b114..388962906829 100644
--- a/vcl/source/outdev/outdevstate.cxx
+++ b/vcl/source/outdev/outdevstate.cxx
@@ -625,7 +625,7 @@ void OutputDevice::ImplReleaseFonts()
if ( mpFontInstance )
{
- mpFontCache->Release( mpFontInstance );
+ mpFontInstance->Release();
mpFontInstance = nullptr;
}
diff --git a/vcl/source/window/window.cxx b/vcl/source/window/window.cxx
index cb6b48077c80..83a3bba5109e 100644
--- a/vcl/source/window/window.cxx
+++ b/vcl/source/window/window.cxx
@@ -1768,7 +1768,7 @@ void Window::ImplNewInputContext()
pFocusWin->ImplGetFrame()->SetInputContext( &aNewContext );
if ( pFontInstance )
- pFocusWin->mpFontCache->Release( pFontInstance );
+ pFontInstance->Release();
}
void Window::doLazyDelete()
diff --git a/vcl/win/gdi/salfont.cxx b/vcl/win/gdi/salfont.cxx
index 9796ab928f3b..001518bf3e55 100644
--- a/vcl/win/gdi/salfont.cxx
+++ b/vcl/win/gdi/salfont.cxx
@@ -890,7 +890,7 @@ void WinSalGraphics::SetFont( const FontSelectPattern* pFont, int nFallbackLevel
mhFonts[ i ] = nullptr;
if (mpWinFontEntry[i])
{
- GetWinFontEntry(i)->mpFontCache->Release(GetWinFontEntry(i));
+ GetWinFontEntry(i)->Release();
}
mpWinFontEntry[i] = nullptr;
mpWinFontData[i] = nullptr;
@@ -902,13 +902,13 @@ void WinSalGraphics::SetFont( const FontSelectPattern* pFont, int nFallbackLevel
assert(pFont->mpFontData);
if (mpWinFontEntry[nFallbackLevel])
{
- GetWinFontEntry(nFallbackLevel)->mpFontCache->Release(GetWinFontEntry(nFallbackLevel));
+ GetWinFontEntry(nFallbackLevel)->Release();
}
// WinSalGraphics::GetEmbedFontData does not set mpFontInstance
// since it is interested in font file data only.
if (pFont->mpFontInstance)
{
- pFont->mpFontInstance->mpFontCache->Acquire(pFont->mpFontInstance);
+ pFont->mpFontInstance->Acquire();
}
mpWinFontEntry[ nFallbackLevel ] = reinterpret_cast<WinFontInstance*>( pFont->mpFontInstance );
mpWinFontData[ nFallbackLevel ] = static_cast<const WinFontFace*>( pFont->mpFontData );
More information about the Libreoffice-commits
mailing list