Bug 86300

Summary: Crash in harfbuzz when destroying ressources
Product: HarfBuzz Reporter: fuhrmann_mail
Component: srcAssignee: Behdad Esfahbod <freedesktop>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: freedesktop, hakuro
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: harfbuzz debug log

Description fuhrmann_mail 2014-11-14 23:02:09 UTC
Current VLC master is producing crashes inside harfbuzz when destroying ressources at the end of file, or when quitting the program. This applies to files with the ass subtitle format, which fires VLCs libass plugin which uses harfbuzz.

Crash log is as below note that both harfbuzz and libass is statically linked into our library. By doing a version "bisect" it can be noticed that harfbuzz version 0.9.17 is fine and the issue started to appear in 0.9.18.




Process 36826 stopped
* thread #5: tid = 0x21fc2b, 0x000000010e52ea9c liblibass_plugin.dylib`OT::BEInt<unsigned short, 2>::operator unsigned short(this=0x0000000117d04570) const + 12 at hb-open-type-private.hh:530, stop reason = EXC_BAD_ACCESS (code=1, address=0x117d04570)
    frame #0: 0x000000010e52ea9c liblibass_plugin.dylib`OT::BEInt<unsigned short, 2>::operator unsigned short(this=0x0000000117d04570) const + 12 at hb-open-type-private.hh:530
   527 	{
   528 	  public:
   529 	  inline void set (Type i) { hb_be_uint16_put (v,i); }
-> 530 	  inline operator Type (void) const { return hb_be_uint16_get (v); }
   531 	  inline bool operator == (const BEInt<Type, 2>& o) const { return hb_be_uint16_eq (v, o.v); }
   532 	  inline bool operator != (const BEInt<Type, 2>& o) const { return !(*this == o); }
   533 	  private: uint8_t v[2];
(lldb) bt
* thread #5: tid = 0x21fc2b, 0x000000010e52ea9c liblibass_plugin.dylib`OT::BEInt<unsigned short, 2>::operator unsigned short(this=0x0000000117d04570) const + 12 at hb-open-type-private.hh:530, stop reason = EXC_BAD_ACCESS (code=1, address=0x117d04570)
  * frame #0: 0x000000010e52ea9c liblibass_plugin.dylib`OT::BEInt<unsigned short, 2>::operator unsigned short(this=0x0000000117d04570) const + 12 at hb-open-type-private.hh:530
    frame #1: 0x000000010e52ea85 liblibass_plugin.dylib`OT::IntType<unsigned short, 2u>::operator unsigned short(this=0x0000000117d04570) const + 21 at hb-open-type-private.hh:561
    frame #2: 0x000000010e5693dc liblibass_plugin.dylib`OT::GenericOffsetTo<OT::Offset, OT::OffsetListOf<OT::Lookup> >::operator(this=0x0000000117d04570, base=0x0000000117d04568)(void const*) const + 28 at hb-open-type-private.hh:690
    frame #3: 0x000000010e569260 liblibass_plugin.dylib`OT::OffsetListOf<OT::Lookup> const& OT::operator+<OT::GSUBGPOS const*, OT::Offset, OT::OffsetListOf<OT::Lookup> >(base=0x0000000105f98ad8, offset=0x0000000117d04570) + 32 at hb-open-type-private.hh:737
    frame #4: 0x000000010e569222 liblibass_plugin.dylib`OT::GSUBGPOS::get_lookup(this=0x0000000117d04568, i=0) const + 50 at hb-ot-layout-gsubgpos-private.hh:2414
    frame #5: 0x000000010e53b2eb liblibass_plugin.dylib`OT::GSUB::get_lookup(this=0x0000000117d04568, i=0) const + 27 at hb-ot-layout-gsub-table.hh:1357
    frame #6: 0x000000010e535b5e liblibass_plugin.dylib`_hb_ot_layout_destroy(layout=0x0000000104f035a0) + 78 at hb-ot-layout.cc:86
    frame #7: 0x000000010e56b695 liblibass_plugin.dylib`_hb_ot_shaper_face_data_destroy(data=0x0000000104f035a0) + 21 at hb-ot-shape.cc:142
    frame #8: 0x000000010e52be1b liblibass_plugin.dylib`hb_face_destroy(face=0x0000000104f02c70) + 203 at hb-shaper-list.hh:39
    frame #9: 0x000000010e52c5e5 liblibass_plugin.dylib`hb_font_destroy(font=0x0000000104f02d40) + 293 at hb-font.cc:872
    frame #10: 0x000000010e51a2cf liblibass_plugin.dylib`ass_shaper_font_data_free(priv=0x0000000104f02ad0) + 63 at ass_shaper.c:126
    frame #11: 0x000000010e50984b liblibass_plugin.dylib`ass_font_free(font=0x000000010e1069e0) + 75 at ass_font.c:619
    frame #12: 0x000000010e506b01 liblibass_plugin.dylib`font_destruct(key=0x000000010e102bb0, value=<unavailable>) + 17 at ass_cache.c:71
    frame #13: 0x000000010e5068bf liblibass_plugin.dylib`ass_cache_done [inlined] ass_cache_empty(cache=<unavailable>, max_size=<unavailable>) + 49 at ass_cache.c:307
    frame #14: 0x000000010e50688e liblibass_plugin.dylib`ass_cache_done(cache=0x00000001006cbd10) + 14 at ass_cache.c:334
    frame #15: 0x000000010e50b199 liblibass_plugin.dylib`ass_renderer_done(render_priv=0x000000010287ea00) + 25 at ass_render.c:179
    frame #16: 0x000000010e50199b liblibass_plugin.dylib`DecSysRelease(p_sys=0x000000010053f330) + 187 at libass.c:289
    frame #17: 0x000000010e502394 liblibass_plugin.dylib`SubpictureDestroy(p_subpic=0x0000000105407890) + 36 at libass.c:496
Comment 1 Behdad Esfahbod 2014-11-15 01:28:45 UTC
Any chance you can bisect further down to find which commit caused it?
Comment 2 fuhrmann_mail 2014-11-15 10:23:03 UTC
I tried a bisect by hand (because of all these buildsystem changes). It turns out https://github.com/behdad/harfbuzz/commit/45fd9424c723f115ca98995b8f8a25185a6fc71d seems to be the first faulty commit.
Comment 3 Behdad Esfahbod 2014-11-18 02:23:36 UTC
There was a bug like that, but was fixed by this:

https://github.com/behdad/harfbuzz/commit/27674b4bb351e501373bd9994e4ba6546e465cf7

which went into 0.9.22.

Can you rewind your bisect and work on 0.9.22 forward only?  Thanks.
Comment 4 fuhrmann_mail 2014-11-18 18:10:04 UTC
I just tried 0.9.22, but the same crash happens with this version.
Comment 5 Behdad Esfahbod 2014-11-24 21:51:59 UTC
Any idea which font causes this?
Comment 6 fuhrmann_mail 2014-11-26 18:53:16 UTC
For testing, you can use the sample linked here: https://trac.videolan.org/vlc/ticket/12733

This sample produces the following ass related debug output, so I assume the font in question is Tamil Sangam MN Bold (they are two different subtitle fonts shown at the same time). 

[ass] [0x10023e880]: Warning: no style named 'testi' found, using 'Default'
[ass] [0x10023e880]: Warning: no style named 'Karaoke' found, using 'Default'
[ass] Neither PlayResX nor PlayResY defined. Assuming 384x288
[ass] fontconfig: cannot find font 'Anime Ace 2.0 BB', falling back to 'Tamil Sangam MN Bold'
[ass] [0x10023e880]: Warning: no style named 'testi' found, using 'Default'

The crash can be reproduced on recent OS X versions, I did not tested windows or linux builds so far.
Comment 7 fuhrmann_mail 2014-11-27 18:01:56 UTC
Then again, I doubt this issue is specific to any font file. We got quite a lot complaints about this crash from various users recently, for instance also with this sample file: https://dl.dropboxusercontent.com/u/48175551/example_subtitles.ass

It only states that normal Arial shall be used.
Comment 8 Behdad Esfahbod 2014-12-02 01:03:57 UTC
Humm.  I have no idea how to proceed.  It's hard to imagine it's an obvious bug in HarfBuzz itself as we don't see it in other integrations.  So someone who can reproduce needs to debug.  Try compiling HarfBuzz with -DHB_DEBUG_OBJECT=100 and see if there's anything suspicious, or paste the log here.  Thanks.
Comment 9 fuhrmann_mail 2014-12-02 18:32:07 UTC
Created attachment 110370 [details]
harfbuzz debug log

Here is a debug log from harfbuzz (with latest hb version). I'm not familiar with the code myself, and the log seems to only contain ref count info. Does this help?
Comment 10 Behdad Esfahbod 2014-12-29 00:06:46 UTC
Ok, I talked to someone else who had a similar issue.  We believe that you might be releasing FT_Face before hb_face_t / hb_font_t.  That definitely explains the problem.  However, I also made a commit that should avoid this problem.  Do fix it on your side though.

commit 395b35903e052aecc97d0807e4f813c64c0d2b0b
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Sun Dec 28 16:03:26 2014 -0800

    Avoid accessing layout tables at face destruction
    
    "Fixes" https://bugs.freedesktop.org/show_bug.cgi?id=86300
    
    Based on discussion someone else who had a similar issue, most probably
    the user is releasing FT_Face before destructing hb_face_t / hb_font_t.
    While that's a client bug, and while we can (and should) use FreeType
    refcounting to help avoid that, it happens that we were accessing
    the table when we didn't really have to.  Avoid that.
Comment 11 Behdad Esfahbod 2014-12-29 00:35:08 UTC
Here's a patch for libass to fix the destruction ordering:

  https://github.com/libass/libass/pull/155
Comment 12 fuhrmann_mail 2014-12-29 17:59:09 UTC
Thanks for the investigation.

I tried both fixes with VLC, and they work fine.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.