From: Simon Arlott <sa.me.uk>
Date: Fri, 11 Dec 2020 18:38:42 +0000 (+0000)
Subject: Use a separate pango fontmap per thread
X-Git-Tag: v1.8.0~33
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=b13bffbaf256fdf27bb0294ac647cdba5c802ffb;p=thirdparty%2Frrdtool-1.x.git

Use a separate pango fontmap per thread

Reusing the same fontmap from multiple threads is not safe, despite
the attempts to use a mutex to protect it there are other instances
where it behaves unexpectedly if graphs are created from multiple
threads at the same time, e.g.:

(process:76234): Pango-WARNING **: 11:47:25.823: failed to create cairo scaled font, expect ugly output. the offending font is 'DejaVu Sans Mono 8'
(process:76234): Pango-WARNING **: 11:47:25.823: font_face status is: no error has occurred
(process:76234): Pango-WARNING **: 11:47:25.823: scaled_font status is: invalid matrix (not invertible)
(process:76234): Pango-WARNING **: 11:47:25.823: shaping failure, expect ugly output. shape-engine='PangoFcShapeEngine', font='DejaVu Sans Mono 8', text='Apparent temp (?C)'

This error results in boxes instead of characters on the graph. In the
worst case scenario there are memory management errors that cause GLib
to complain or the process to abort.

Calling pango_cairo_font_map_get_default() multiple times from the same
thread will return the same object, so this satisfies the performance
expectations of reusing it within a single thread. Trying to lock a
mutex for even more pango/cairo function calls is likely to decrease
performance when multiple threads are used concurrently.
---

diff --git a/src/rrd_graph.c b/src/rrd_graph.c
index 59bb5825..c0ce6d46 100644
--- a/src/rrd_graph.c
+++ b/src/rrd_graph.c
@@ -485,11 +485,9 @@ int im_free(
             free(im->rendered_image);
         }
 
-        mutex_lock(im->fontmap_mutex);
         if (im->layout) {
             g_object_unref(im->layout);
         }
-        mutex_unlock(im->fontmap_mutex);
     }
 
     if (im->ylegend)
@@ -4822,8 +4820,6 @@ void rrd_graph_init(
 {
     unsigned int i;
     char     *deffont = getenv("RRD_DEFAULT_FONT");
-    static PangoFontMap *fontmap = NULL;
-    static mutex_t fontmap_mutex = MUTEX_INITIALIZER;
     PangoContext *context;
 
     /* zero the whole structure first */
@@ -4913,7 +4909,6 @@ void rrd_graph_init(
         im->font_options = cairo_font_options_create();
         im->surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, 10, 10);
         im->cr = cairo_create(im->surface);
-        im->fontmap_mutex = &fontmap_mutex;
 
         for (i = 0; i < DIM(text_prop); i++) {
             im->text_prop[i].size = -1;
@@ -4922,11 +4917,7 @@ void rrd_graph_init(
                               text_prop[i].size);
         }
 
-        mutex_lock(im->fontmap_mutex);
-
-        if (fontmap == NULL) {
-            fontmap = pango_cairo_font_map_new();
-        }
+        PangoFontMap *fontmap = pango_cairo_font_map_get_default();
 
 #ifdef HAVE_PANGO_FONT_MAP_CREATE_CONTEXT
         context = pango_font_map_create_context((PangoFontMap *) fontmap);
@@ -4950,8 +4941,6 @@ void rrd_graph_init(
             (im->font_options, CAIRO_HINT_METRICS_ON);
         cairo_font_options_set_antialias(im->font_options,
                                          CAIRO_ANTIALIAS_GRAY);
-
-        mutex_unlock(im->fontmap_mutex);
     }
 
     for (i = 0; i < DIM(graph_col); i++)
@@ -5585,11 +5574,9 @@ void rrd_graph_options(
         }
     }                   /* while (opt != -1) */
 
-    mutex_lock(im->fontmap_mutex);
     pango_cairo_context_set_font_options(pango_layout_get_context(im->layout),
                                          im->font_options);
     pango_layout_context_changed(im->layout);
-    mutex_unlock(im->fontmap_mutex);
 
 
     if (im->primary_axis_format != NULL && im->primary_axis_format[0] != '\0') {
diff --git a/src/rrd_graph.h b/src/rrd_graph.h
index 914695f3..4df32ec6 100644
--- a/src/rrd_graph.h
+++ b/src/rrd_graph.h
@@ -25,7 +25,6 @@
 
 #include "rrd_tool.h"
 #include "rrd_rpncalc.h"
-#include "mutex.h"
 
 #include <glib.h>
 
@@ -353,7 +352,6 @@ typedef struct image_desc_t {
     rrd_info_t *grinfo_current; /* pointing to current entry */
     GHashTable* gdef_map;  /* a map of all *def gdef entries for quick access */
     GHashTable* rrd_map;  /* a map of all rrd files in use for gdef entries */
-    mutex_t *fontmap_mutex; /* Mutex for locking the global fontmap */
     enum image_init_en init_mode; /* do we need Cairo/Pango? */
     double x_pixie; /* scale for X (see xtr() for reference) */
     double y_pixie; /* scale for Y (see ytr() for reference) */