[PATCH libreoffice-4-0] Resolves: rhbz#963276 rhbz#761009 lp#766153 lp#892904 Handle...

Caolán McNamara (via_Code_Review) gerrit at gerrit.libreoffice.org
Thu May 16 07:07:57 PDT 2013


Hi,

I have submitted a patch for review:

    https://gerrit.libreoffice.org/3922

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/core refs/changes/22/3922/1

Resolves: rhbz#963276 rhbz#761009 lp#766153 lp#892904 HandleFontOptions crash

The mpServerFont member of a ImplServerFontEntry must not be deleted while the
ImplServerFontEntry still exists

see also 39cbce553da1834f78b77f48b2f1be9578d6cc05 for another reason a crash in
the same place can happen.  Its impossible from traces in crashes before
39cbce553da1834f78b77f48b2f1be9578d6cc05 was fixed to distinguish those crashes
from this crash.

This crash is a regression due to 7a416820ab5e03f8b988656e0f6a592cb1e81d07
where we went from modifying pServerFont in X11SalGraphics::setFont directly to
modifying it/a-different-one indirectly via ImplServerFontEntry

The various font caches and font thing lifecycles of LibreOffice are somewhat
confusing.

This crash had eluded me for years, to reproduce:
insert->special chars->select a font with loads of glyphs, i.e. "AR PL UKai CN"
click on the first row of glyphs and hold down page-down until you hit the
bottom, then page-up until you hit the top. Pre patch it won't survive the
whole down+up (and valgrind will moan quite a bit)

(cherry picked from commit 3d6424a752902bba513102834966868f609d91e6)

Change-Id: Ifde0cb375f487c556b04a640d77765a7dc2f0913
---
M vcl/generic/glyphs/glyphcache.cxx
M vcl/inc/generic/glyphcache.hxx
M vcl/unx/generic/gdi/salgdi3.cxx
3 files changed, 19 insertions(+), 5 deletions(-)



diff --git a/vcl/generic/glyphs/glyphcache.cxx b/vcl/generic/glyphs/glyphcache.cxx
index bc3c544..826b4cc 100644
--- a/vcl/generic/glyphs/glyphcache.cxx
+++ b/vcl/generic/glyphs/glyphcache.cxx
@@ -410,9 +410,22 @@
 
 // -----------------------------------------------------------------------
 
+void ImplServerFontEntry::SetServerFont(ServerFont* p)
+{
+    if (p == mpServerFont)
+        return;
+    if (mpServerFont)
+        mpServerFont->Release();
+    mpServerFont = p;
+    if (mpServerFont)
+        mpServerFont->AddRef();
+}
+
 ImplServerFontEntry::~ImplServerFontEntry()
 {
     // TODO: remove the ServerFont here instead of in the GlyphCache
+    if (mpServerFont)
+        mpServerFont->Release();
 }
 
 // =======================================================================
diff --git a/vcl/inc/generic/glyphcache.hxx b/vcl/inc/generic/glyphcache.hxx
index 655f2cd..14af1d3 100644
--- a/vcl/inc/generic/glyphcache.hxx
+++ b/vcl/inc/generic/glyphcache.hxx
@@ -224,6 +224,7 @@
 private:
     friend class GlyphCache;
     friend class ServerFontLayout;
+    friend class ImplServerFontEntry;
     friend class X11SalGraphics;
 
     void                        AddRef() const      { ++mnRefCount; }
@@ -301,7 +302,7 @@
 public:
                    ImplServerFontEntry( FontSelectPattern& );
     virtual        ~ImplServerFontEntry();
-    void           SetServerFont( ServerFont* p) { mpServerFont = p; }
+    void           SetServerFont(ServerFont* p);
     void           HandleFontOptions();
 };
 
diff --git a/vcl/unx/generic/gdi/salgdi3.cxx b/vcl/unx/generic/gdi/salgdi3.cxx
index beaf8fd..21089cc 100644
--- a/vcl/unx/generic/gdi/salgdi3.cxx
+++ b/vcl/unx/generic/gdi/salgdi3.cxx
@@ -194,10 +194,10 @@
 
         // apply font specific-hint settings if needed
         // TODO: also disable it for reference devices
-    if( !bPrinter_ )
-    {
-        ImplServerFontEntry* pSFE = static_cast<ImplServerFontEntry*>( pEntry->mpFontEntry );
-        pSFE->HandleFontOptions();
+        if( !bPrinter_ )
+        {
+            ImplServerFontEntry* pSFE = static_cast<ImplServerFontEntry*>( pEntry->mpFontEntry );
+            pSFE->HandleFontOptions();
         }
 
         return true;

-- 
To view, visit https://gerrit.libreoffice.org/3922
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifde0cb375f487c556b04a640d77765a7dc2f0913
Gerrit-PatchSet: 1
Gerrit-Project: core
Gerrit-Branch: libreoffice-4-0
Gerrit-Owner: Caolán McNamara <caolanm at redhat.com>



More information about the LibreOffice mailing list