Thread-unsafe cairo_font_face_t is shared between multiple threads in CairoFontEngine

There are 2 caching mechanisms in CairoFontEngine.cc. One in CairoFontEngine::getFont with cache size of cairoFontCacheSize (64) which is protected by mutex and local to CairoFontEngine instance as CairoFontEngine::fontCache. Other is in _ft_new_face and it is global linked list _ft_open_faces not protected by any additional mutex and not limited in size so it could possibly grow indefinitely. Maybe would be enough to have just one cache. There is problem with both of them.

New independent instance of CairoFontEngine is created for each call to poppler_document_new_from_file so should be safe to open documents in multiple threads. But not so safe to render multiple pages from opened document in multiple threads because then they could share same cairo_font_face_t. As there is mutex in CairoFontEngine::getFont I would assume that one instance of CairoFontEngine is meant to be used from multiple thread but it cannot be.

Problem with _ft_open_faces is that it is shared globally so even opening independent documents in multiple threads and rendering them is unsafe.

Attached test_mt_render.zip

This ends up like this:

a.out: ../../../../src/cairo-scaled-font.c:1326: cairo_scaled_font_destroy: Assertion `! scaled_font->cache_frozen' failed.

#4  0x00007ffff75abed1 in INT_cairo_scaled_font_destroy (scaled_font=scaled_font@entry=0x7fffd0050210) at ../../../../src/cairo-scaled-font.c:1326
#5  0x00007ffff75acc7a in INT_cairo_scaled_font_destroy (scaled_font=scaled_font@entry=0x7fffd0050210) at ../../../../src/cairo-scaled-font.c:1317
#6  0x00007ffff7baf986 in CairoFont::getSubstitutionCorrection(GfxFont*) (this=<optimized out>, gfxFont=0x7fffc004a880) at ./poppler/CairoFontEngine.cc:142
#7  0x00007ffff7bb11e3 in CairoOutputDev::updateFont(GfxState*) (this=0x7fffc00369d0, state=0x7fffc003a710) at ./poppler/CairoOutputDev.cc:684
#8  0x00007ffff6614922 in Gfx::opShowText(Object*, int) () at /usr/lib/x86_64-linux-gnu/libpoppler.so.73

Or this:

a.out: ../../../../src/cairo-scaled-font.c:1326: cairo_scaled_font_destroy: Assertion `! scaled_font->cache_frozen' failed.

#4  0x00007ffff75abed1 in INT_cairo_scaled_font_destroy (scaled_font=scaled_font@entry=0x7fffe0058d00) at ../../../../src/cairo-scaled-font.c:1326
#5  0x00007ffff75ac99d in INT_cairo_scaled_font_destroy (scaled_font=0x7fffe0058d00) at ../../../../src/cairo-scaled-font.c:1317
#6  0x00007ffff75ac99d in INT_cairo_scaled_font_create (font_face=<optimized out>, font_matrix=font_matrix@entry=0x7fffcc0343d0, ctm=ctm@entry=0x7fffecad4080, options=options@entry=0x7fffecad4060) at ../../../../src/cairo-scaled-font.c:1118
#7  0x00007ffff757239e in _cairo_gstate_ensure_scaled_font (gstate=0x7fffcc034360) at ../../../../src/cairo-gstate.c:1915
#8  0x00007ffff7575464 in _cairo_gstate_show_text_glyphs (gstate=0x7fffcc034360, glyphs=0x7fffcc07e490, num_glyphs=<optimized out>, info=0x0) at ../../../../src/cairo-gstate.c:2004
#9  0x00007ffff7567344 in cairo_show_glyphs (cr=0x7fffcc037610, glyphs=<optimized out>, num_glyphs=<optimized out>) at ../../../../src/cairo.c:3630
#10 0x00007ffff7bb2cd9 in CairoOutputDev::endString(GfxState*) (this=0x7fffcc0365c0, state=<optimized out>) at ./poppler/CairoOutputDev.cc:1461
#11 0x00007ffff6613d19 in Gfx::doShowText(GooString*) () at /usr/lib/x86_64-linux-gnu/libpoppler.so.73

I am getting this almost always instantly after run. Tested with poppler 0.62.0 on ubuntu 18.04 (and poppler 0.48.0 on debian stretch) on cpu with 12/24 cores. On other systems it may need more time or to be run multiple times. It can end up with segmentation fault or different abort but that is another bug.

It is happening because CairoFont::getSubstitutionCorrection in one thread uses same cairo_font_face_t as CairoOutputDev::endString in another thread:

CairoFont::getSubstitutionCorrection:

	cairo_scaled_font_t *scaled_font = cairo_scaled_font_create(cairo_font_face, &m, &m, options);

	cairo_text_extents_t extents;
	cairo_scaled_font_text_extents(scaled_font, "m", &extents);

	cairo_scaled_font_destroy(scaled_font);

CairoOutputDev::endString:

    else
        cairo_show_glyphs (cairo, glyphs, glyphCount);
    if (cairo_shape)
      cairo_show_glyphs (cairo_shape, glyphs, glyphCount);

Only cairo_scaled_font_t is thread-safe as is written in sources:

struct _cairo_scaled_font {
    /* For most cairo objects, the rule for multiple threads is that
     * the user is responsible for any locking if the same object is
     * manipulated from multiple threads simultaneously.
     *
     * However, with the caching that cairo does for scaled fonts, a
...
     * So, as a special exception, the cairo implementation takes care
     * of all locking needed for cairo_scaled_font_t. Most of what is

But cairo_font_face_t is not thread-safe.

Following program is doing roughly what is doing poppler in test_mt_render.zip (including most recent version of poppler):

#include <vector>
#include <thread>
#include <cassert>
#include <cairo.h>
#include <cairo-ft.h>

int main()
{
	FT_Library lib;
	FT_Init_FreeType(&lib);
	FT_Face face;
	assert(!FT_New_Face(lib, "/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf", 0, &face));
	cairo_font_face_t *font_face = cairo_ft_font_face_create_for_ft_face(face, FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP);

	std::vector<std::thread> threads;
	for(int t = 0; t < std::thread::hardware_concurrency() / 2; t++)
		threads.emplace_back([font_face]()
		{
			cairo_surface_t *surface = cairo_image_surface_create(CAIRO_FORMAT_RGB24, 100, 100);
			cairo_t *ctx = cairo_create(surface);
			cairo_matrix_t m;
			cairo_matrix_init_identity(&m);
			cairo_font_options_t *options = cairo_font_options_create();

			while(true)
			{
				cairo_scaled_font_t *scaled_font = cairo_scaled_font_create(font_face, &m, &m, options);
				cairo_text_extents_t extents;
				cairo_scaled_font_text_extents(scaled_font, "m", &extents);
				cairo_scaled_font_destroy(scaled_font);

				cairo_set_font_face(ctx, font_face);
				cairo_set_font_size(ctx, 24);
				cairo_show_text(ctx, "Text");
			}
		});
	for(auto &th : threads)th.join();
	return 0;
}

With similar aborts and stack traces in GDB (if you happen to have fonts-dejavu-core).