Bug 247628 – [GTK] Crash in WebCore::Cairo::drawGlyphs if threaded rendering is enabled

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Michael Catanzaro]

Created attachment 463457 [details]
Full backtrace, all threads

Core was generated by `/usr/libexec/webkit2gtk-5.0/WebKitWebProcess 1954 146'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  FT_Load_Glyph (face=face@entry=0x555aca110030, glyph_index=17, load_flags=load_flags@entry=1114624) at ../src/base/ftobjs.c:1108
1108	      slot->linearHoriAdvance = FT_MulDiv( slot->linearHoriAdvance,

This looks like it might be due to switch to Nicosia threaded rendering? The crash thread looks like this:

Thread 1 (Thread 0x7f5a5ffff640 (LWP 87)):
#0  FT_Load_Glyph (face=face@entry=0x555aca110030, glyph_index=17, load_flags=load_flags@entry=1114624) at ../src/base/ftobjs.c:1108
#1  0x00007f5d2cc28867 in _cairo_ft_scaled_glyph_load_glyph (scaled_font=scaled_font@entry=0x7f5a54001db0, scaled_glyph=scaled_glyph@entry=0x7f5a54003910, face=face@entry=0x555aca110030, load_flags=load_flags@entry=1114624, use_em_size=use_em_size@entry=0, vertical_layout=vertical_layout@entry=0) at ../../src/cairo-ft-font.c:2428
#2  0x00007f5d2cc2b6f9 in _cairo_ft_scaled_glyph_init (abstract_font=0x7f5a54001db0, scaled_glyph=0x7f5a54003910, info=CAIRO_SCALED_GLYPH_INFO_METRICS) at ../../src/cairo-ft-font.c:2501
#3  0x00007f5d2cbcd2ea in _cairo_scaled_glyph_lookup (scaled_font=0x7f5a54001db0, index=<optimized out>, info=CAIRO_SCALED_GLYPH_INFO_METRICS, scaled_glyph_ret=0x7f5a5fffce58) at ../../src/cairo-scaled-font.c:3017
#4  0x00007f5d2cbce617 in _cairo_scaled_font_single_glyph_device_extents (extents=0x7f5a5fffd18c, glyph=0x7f5a5fffd8c0, scaled_font=0x7f5a54001db0) at ../../src/cairo-scaled-font.c:2176
#5  _cairo_scaled_font_glyph_device_extents (scaled_font=scaled_font@entry=0x7f5a54001db0, glyphs=glyphs@entry=0x7f5a5fffd8c0, num_glyphs=num_glyphs@entry=1, extents=extents@entry=0x7f5a5fffd18c, overlap_out=overlap_out@entry=0x7f5a5fffd16c) at ../../src/cairo-scaled-font.c:2228
#6  0x00007f5d2cb89e6b in _cairo_composite_rectangles_init_for_glyphs (extents=extents@entry=0x7f5a5fffd170, surface=<optimized out>, op=<optimized out>, source=<optimized out>, scaled_font=scaled_font@entry=0x7f5a54001db0, glyphs=glyphs@entry=0x7f5a5fffd8c0, num_glyphs=<optimized out>, clip=<optimized out>, overlap=<optimized out>) at ../../src/cairo-composite-rectangles.c:446
#7  0x00007f5d2cb8a4bd in _cairo_compositor_glyphs (compositor=0x7f5d2cc868e0 <spans>, surface=<optimized out>, op=<optimized out>, source=<optimized out>, glyphs=0x7f5a5fffd8c0, num_glyphs=1, scaled_font=<optimized out>, clip=<optimized out>) at ../../src/cairo-compositor.c:238
#8  0x00007f5d2cb9d40a in _cairo_image_surface_glyphs (abstract_surface=<optimized out>, op=<optimized out>, source=<optimized out>, glyphs=<optimized out>, num_glyphs=<optimized out>, scaled_font=<optimized out>, clip=<optimized out>) at ../../src/cairo-image-surface.c:1007
#9  0x00007f5d2cbdee7e in _cairo_surface_show_text_glyphs (surface=0x7f5a54000c80, op=CAIRO_OPERATOR_OVER, source=source@entry=0x7f5a5fffd570, utf8=<optimized out>, utf8_len=<optimized out>, glyphs=<optimized out>, num_glyphs=<optimized out>, clusters=<optimized out>, num_clusters=<optimized out>, cluster_flags=<optimized out>, scaled_font=<optimized out>, clip=<optimized out>) at ../../src/cairo-surface.c:2891
#10 0x00007f5d2cb94428 in _cairo_gstate_show_text_glyphs (gstate=0x7f5a54001b00, glyphs=<optimized out>, num_glyphs=<optimized out>, info=0x0) at ../../src/cairo-gstate.c:2077
#11 0x00007f5d2cbed04c in cairo_show_glyphs (cr=0x7f5a54001550, glyphs=<optimized out>, num_glyphs=<optimized out>) at ../../src/cairo.c:3630
#12 0x00007f5d31d005f0 in WebCore::Cairo::drawGlyphs(WebCore::GraphicsContextCairo&, WebCore::Cairo::FillSource const&, WebCore::Cairo::StrokeSource const&, WebCore::Cairo::ShadowState const&, WebCore::FloatPoint const&, _cairo_scaled_font*, double, WTF::Vector<cairo_glyph_t, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, float, WTF::OptionSet<WebCore::TextDrawingMode>, float, WebCore::FloatSize const&, WebCore::Color const&, WebCore::FontSmoothingMode) (platformContext=<optimized out>, fillSource=<optimized out>, strokeSource=..., shadowState=<optimized out>, point=<optimized out>, scaledFont=0x555aca13bf10, syntheticBoldOffset=<optimized out>, glyphs=<optimized out>, xOffset=<optimized out>, textDrawingMode=..., strokeThickness=<optimized out>, shadowOffset=<optimized out>, shadowColor=<optimized out>, fontSmoothingMode=<optimized out>) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:840
#13 0x00007f5d305f3167 in DrawGlyphs::execute(Nicosia::PaintingOperationReplay&) (this=<optimized out>, replayer=<optimized out>) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/nicosia/cairo/NicosiaCairoOperationRecorder.cpp:514
#14 0x00007f5d305fbd50 in Nicosia::PaintingContextCairo::ForPainting::replay(WTF::Vector<std::unique_ptr<Nicosia::PaintingOperation, std::default_delete<Nicosia::PaintingOperation> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&) (this=<optimized out>, paintingOperations=<optimized out>) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/nicosia/cairo/NicosiaPaintingContextCairo.cpp:97
#15 0x00007f5d305f009a in Nicosia::PaintingContext::replay(Nicosia::Buffer&, WTF::Vector<std::unique_ptr<Nicosia::PaintingOperation, std::default_delete<Nicosia::PaintingOperation> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&) (paintingOperations=..., buffer=<optimized out>) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/nicosia/NicosiaPaintingContext.h:62
#16 operator() (__closure=0x7f5a514c6bc8) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/nicosia/NicosiaPaintingEngineThreaded.cpp:83
#17 WTF::Detail::CallableWrapper<Nicosia::PaintingEngineThreaded::paint(WebCore::GraphicsLayer&, WTF::Ref<Nicosia::Buffer>&&, const WebCore::IntRect&, const WebCore::IntRect&, const WebCore::IntRect&, float)::<lambda()>, void>::call(void) (this=0x7f5a514c6bc0) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/_builddir/WTF/Headers/wtf/Function.h:53
#18 0x00007f5d2f066b9a in WTF::Function<void ()>::operator()() const (this=0x7f5d251582b8) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WTF/wtf/Function.h:82
#19 WTF::WorkerPool::Worker::work() (this=0x7f5d25158280) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WTF/wtf/WorkerPool.cpp:53
#20 0x00007f5d2efff7eb in operator() (__closure=0x7f5d25117688) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WTF/wtf/AutomaticThread.cpp:229
#21 WTF::Detail::CallableWrapper<WTF::AutomaticThread::start(const WTF::AbstractLocker&)::<lambda()>, void>::call(void) (this=0x7f5d25117680) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WTF/wtf/Function.h:53
#22 0x00007f5d2f02fa05 in WTF::Function<void ()>::operator()() const (this=<synthetic pointer>) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WTF/wtf/Function.h:79
#23 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (newThreadContext=0x7f5d2511e890) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WTF/wtf/Threading.cpp:236
#24 0x00007f5d2f090d9d in WTF::wtfThreadEntryPoint(void*) (context=<optimized out>) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WTF/wtf/posix/ThreadingPOSIX.cpp:242
#25 0x00007f5d2f56e1da in start_thread (arg=<optimized out>) at pthread_create.c:442
#26 0x00007f5d2f5f6d84 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100

That looks pretty deep in cairo, so it's probably a cairo bug. However, I notice that the crash occurs in cairo font code, and a different thread is also executing cairo font code at the time of the crash:

Thread 35 (Thread 0x7f5d28a8a100 (LWP 2)):
#0  0x00007f5d2f5e6ba1 in __libc_open64 (file=file@entry=0x555aca16dbc0 "/run/host/fonts/liberation-serif/LiberationSerif-Regular.ttf", oflag=oflag@entry=0) at ../sysdeps/unix/sysv/linux/open64.c:41
#1  0x00007f5d2c70ffc0 in open64 (__oflag=0, __path=0x555aca16dbc0 "/run/host/fonts/liberation-serif/LiberationSerif-Regular.ttf") at /usr/include/x86_64-linux-gnu/bits/fcntl2.h:53
#2  FT_Stream_Open (stream=0x555aca10f9f0, filepathname=0x555aca16dbc0 "/run/host/fonts/liberation-serif/LiberationSerif-Regular.ttf") at ../builds/unix/ftsystem.c:258
#3  0x00007f5d2c69a6f7 in FT_Stream_New (library=library@entry=0x555ac9fddba0, args=args@entry=0x7ffcd92b1890, astream=astream@entry=0x7ffcd92b16a0) at ../src/base/ftobjs.c:238
#4  0x00007f5d2c6a06fb in ft_open_face_internal (library=0x555ac9fddba0, args=args@entry=0x7ffcd92b1890, face_index=0, aface=aface@entry=0x7ffcd92b1900, test_mac_fonts=test_mac_fonts@entry=1 '\001') at ../src/base/ftobjs.c:2562
#5  0x00007f5d2c6a1272 in FT_New_Face (library=<optimized out>, pathname=<optimized out>, face_index=<optimized out>, aface=aface@entry=0x7ffcd92b1900) at ../src/base/ftobjs.c:1605
#6  0x00007f5d2cc29f34 in _cairo_ft_unscaled_font_lock_face (unscaled=0x555ac9ddb040) at ../../src/cairo-ft-font.c:733
#7  0x00007f5d2cc2cb97 in cairo_ft_scaled_font_lock_face (abstract_font=0x555aca231820) at ../../src/cairo-ft-font.c:3849
#8  0x00007f5d30557f86 in WebCore::CairoFtFaceLocker::CairoFtFaceLocker(_cairo_scaled_font*) (scaledFont=0x555aca231820, this=<synthetic pointer>) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/cairo/CairoUtilities.h:52
#9  WebCore::ComplexTextController::collectComplexTextRunsForCharacters(char16_t const*, unsigned int, unsigned int, WebCore::Font const*) (this=0x7ffcd92b1d20, characters=0x7f5d25c67584 u"x86 registers\xffff", length=13, stringLocation=0, font=0x7f5d1d03c800) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:510
#10 0x00007f5d31c5d028 in WebCore::ComplexTextController::collectComplexTextRuns() (this=0x7ffcd92b1d20) at /usr/lib/debug/source/sdk/webkit2gtk-5.0.bst/Source/WebCore/platform/graphics/ComplexTextController.cpp:442

In theory, that's not necessarily unsafe because there does not appear to be any objects shared across threads... but it's a little suspicious. Full backtrace attached.

[Carlos Garcia Campos]

I bet this is a threaded rendering bug. I think we should just disable threaded rendering in gtk4 by default, because cairo is not supposed to be thread safe.

[Miguel Gomez]

(In reply to Carlos Garcia Campos from comment #1)
> I bet this is a threaded rendering bug. I think we should just disable
> threaded rendering in gtk4 by default, because cairo is not supposed to be
> thread safe.

Agree. I've been the main supporter of this feature because it's an important performance boost on embedded devices. Until now I hadn't received any single report of problems caused by it, so I thought it was safe. But I see that it's not reliable as I thought, so it's better to remove it.

[Michael Catanzaro]

I suspect bug #246460 may also be caused by threaded rendering, but I'm not sure.

(In reply to Carlos Garcia Campos from comment #1)
> I bet this is a threaded rendering bug. I think we should just disable
> threaded rendering in gtk4 by default, because cairo is not supposed to be
> thread safe.

Is it unsafe even if no objects are not shared across threads? :/

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #1)
> I bet this is a threaded rendering bug. I think we should just disable
> threaded rendering in gtk4 by default, because cairo is not supposed to be
> thread safe.

This code does not look like it's GTK-specific. This is probably a problem for WPE too, right?

[Miguel Gomez]

(In reply to Michael Catanzaro from comment #4)
> (In reply to Carlos Garcia Campos from comment #1)
> > I bet this is a threaded rendering bug. I think we should just disable
> > threaded rendering in gtk4 by default, because cairo is not supposed to be
> > thread safe.
> 
> This code does not look like it's GTK-specific. This is probably a problem
> for WPE too, right?

It's not enabled by default for WPE, except for downstream deployments.

[Calvin Walton]

I'm curious, what cairo version is this issue happening with?

FWIW, the old stable cairo 1.16.0 has a bunch of issues in the glyph cache code which were causing webfont related crashes or hangs for me with webkit gtk3/epiphany and poppler - there's been a bunch of fixes in the cairo development branch and 1.17.6 prerelease (to the point where several linux distributions have switched to packaging the prerelease).

[Michael Catanzaro]

Using Ephy Tech Preview, which has cairo 1.16.0 with some CVE patches: https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/blob/82a49865caa59851ce1f112a0867e6a1da9c29c6/elements/components/cairo.bst

It's good to know that cairo bugs are being fixed, but unlike the distros that picked up the 1.17 releases, we do not take prerelease updates.

[Carlos Garcia Campos]

I guess this is no longer happening since we disabled threaded rendering, right?

[Michael Catanzaro]

Probably, yes. I already forgot about this bug. If it was still a serious problem, you would hear me whining about it for sure.

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #1)
> I bet this is a threaded rendering bug. I think we should just disable
> threaded rendering in gtk4 by default, because cairo is not supposed to be
> thread safe.

Hey bad news, I just hit this crash again today after upgrading to 2.39.90. I'll attach an all-threads backtrace but it shows cairo_ft being used on both the web process main thread and at the same time on the lone Nicosia painting thread, so it seems that using one painting thread was not as safe as you had hoped.

[Michael Catanzaro]

Created attachment 465136 [details]
Backtrack with 2.39.90

[Carlos Garcia Campos]

The problem is that while recording, we still use cairo for text layout, even if we don't do any rendering. If the rendering thread is using freetype at the same time the main thread is doing the text layout while recording we end up using freetype from 2 threads which is not supported. I think we could try to add a lock somewhere to prevent this.

[Carlos Garcia Campos]

Created attachment 465277 [details]
Patch

Maybe something like this is enough. I think we will need it for offscreen canvas to work too.

[Carlos Garcia Campos]

Pull request: https://github.com/WebKit/WebKit/pull/11102

[EWS]

Committed 261318@main (b59118c2ad0b): <https://commits.webkit.org/261318@main>

Reviewed commits have been landed. Closing PR #11102 and removing active labels.