Bug 157132 – [FreeType] ASSERTION FAILED: !lookupForWriting(Extractor::extract(entry)).second in FontCache::getVerticalData()

• Component: Platform
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

I've noticed that some tests fail randomly in the GTK+ debug bot due to an assertion in HashMap when getting vertical data from the FontCache. I don't know exactly what's wrong, but looks alike a problem with the FontVerticalDataCache hash traits implementation. Looking at the code, I've realized that we could simplify everything by reusing the FontDataCache hash and traits, since we are actually using the FontPlatformData::hash() in the end in both cases. Also, I don't see why we need to get the vertical data from the FontPlatformData while it's actually cached by the font cache. We could just using the FontCache directly passing only the FontPlatformData. These changes seem to fix the crashes and make the code a lot simpler.

See the full bt:

STDERR: ASSERTION FAILED: !lookupForWriting(Extractor::extract(entry)).second
STDERR: ../../Source/WTF/wtf/HashTable.h(985) : WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType* WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::reinsert(WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType&&) [with Key = WTF::AtomicString; Value = WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >; Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >; HashFunctions = WebCore::FontVerticalDataCacheKeyHash; Traits = WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits>::KeyValuePairTraits; KeyTraits = WebCore::FontVerticalDataCacheKeyTraits; WTF::HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::ValueType = WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >]
STDERR: 1   0x7fbc12a27fcd /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7fbc12a27fcd]
STDERR: 2   0x7fbc19e2b07f /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::HashTable<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits>::reinsert(WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >&&)+0x89) [0x7fbc19e2b07f]
STDERR: 3   0x7fbc19e2a2b5 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::HashTable<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits>::rehash(unsigned int, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >*)+0xf1) [0x7fbc19e2a2b5]
STDERR: 4   0x7fbc19e28cad /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::HashTable<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits>::expand(WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >*)+0x61) [0x7fbc19e28cad]
STDERR: 5   0x7fbc19e27268 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::HashTableAddResult<WTF::HashTableIterator<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits> > WTF::HashTable<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits>::add<WTF::HashMapTranslator<WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyHash>, WTF::AtomicString const&, WTF::RefPtr<WebCore::OpenTypeVerticalData>&>(WTF::AtomicString const&, WTF::RefPtr<WebCore::OpenTypeVerticalData>&)+0x27c) [0x7fbc19e27268]
STDERR: 6   0x7fbc19e24d26 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::HashTableAddResult<WTF::HashTableIterator<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits> > WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::inlineAdd<WTF::AtomicString const&, WTF::RefPtr<WebCore::OpenTypeVerticalData>&>(WTF::AtomicString const&, WTF::RefPtr<WebCore::OpenTypeVerticalData>&)+0x4a) [0x7fbc19e24d26]
STDERR: 7   0x7fbc19e22836 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::HashTableAddResult<WTF::HashTableIterator<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits> > WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::inlineSet<WTF::AtomicString const&, WTF::RefPtr<WebCore::OpenTypeVerticalData>&>(WTF::AtomicString const&, WTF::RefPtr<WebCore::OpenTypeVerticalData>&)+0x4a) [0x7fbc19e22836]
STDERR: 8   0x7fbc19e202bf /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WTF::HashTableAddResult<WTF::HashTableIterator<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData> > >, WebCore::FontVerticalDataCacheKeyHash, WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::KeyValuePairTraits, WebCore::FontVerticalDataCacheKeyTraits> > WTF::HashMap<WTF::AtomicString, WTF::RefPtr<WebCore::OpenTypeVerticalData>, WebCore::FontVerticalDataCacheKeyHash, WebCore::FontVerticalDataCacheKeyTraits, WTF::HashTraits<WTF::RefPtr<WebCore::OpenTypeVerticalData> > >::set<WTF::RefPtr<WebCore::OpenTypeVerticalData>&>(WTF::AtomicString const&, WTF::RefPtr<WebCore::OpenTypeVerticalData>&)+0x3b) [0x7fbc19e202bf]
STDERR: 9   0x7fbc19e1d566 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontCache::getVerticalData(WTF::AtomicString const&, WebCore::FontPlatformData const&)+0x144) [0x7fbc19e1d566]
STDERR: 10  0x7fbc1a7446a5 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontPlatformData::verticalData() const+0x95) [0x7fbc1a7446a5]
STDERR: 11  0x7fbc19e0f0da /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::Font::Font(WebCore::FontPlatformData const&, bool, bool, bool)+0x272) [0x7fbc19e0f0da]
STDERR: 12  0x7fbc1945cbf1 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::Font::create(WebCore::FontPlatformData const&, bool, bool, bool)+0x4c) [0x7fbc1945cbf1]
STDERR: 13  0x7fbc19e1d7e3 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontCache::fontForPlatformData(WebCore::FontPlatformData const&)+0x61) [0x7fbc19e1d7e3]
STDERR: 14  0x7fbc19e1d73b /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontCache::fontForFamily(WebCore::FontDescription const&, WTF::AtomicString const&, WebCore::FontFeatureSettings const*, WebCore::FontVariantSettings const*, bool)+0xad) [0x7fbc19e1d73b]
STDERR: 15  0x7fbc1947643b /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::CSSFontSelector::fontRangesForFamily(WebCore::FontDescription const&, WTF::AtomicString const&)+0x10f) [0x7fbc1947643b]
STDERR: 16  0x7fbc19e3f4dc /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0x61f34dc) [0x7fbc19e3f4dc]
STDERR: 17  0x7fbc19e3f892 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontCascadeFonts::realizeFallbackRangesAt(WebCore::FontCascadeDescription const&, unsigned int)+0x18e) [0x7fbc19e3f892]
STDERR: 18  0x7fbc19997c9a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontCascadeFonts::primaryFont(WebCore::FontCascadeDescription const&)+0x68) [0x7fbc19997c9a]
STDERR: 19  0x7fbc19997d88 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontCascade::primaryFont() const+0x64) [0x7fbc19997d88]
STDERR: 20  0x7fbc19997d1a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::FontCascade::fontMetrics() const+0x18) [0x7fbc19997d1a]
STDERR: 21  0x7fbc1a245f80 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderStyle::fontMetrics() const+0x28) [0x7fbc1a245f80]
STDERR: 22  0x7fbc1a246276 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderStyle::computedLineHeight() const+0x46) [0x7fbc1a246276]
STDERR: 23  0x7fbc19fb42d6 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderBlock::lineHeight(bool, WebCore::LineDirectionMode, WebCore::LinePositionMode) const+0x144) [0x7fbc19fb42d6]
STDERR: 24  0x7fbc19fb4bc8 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderBlock::minLineHeightForReplacedRenderer(bool, WebCore::LayoutUnit) const+0xe8) [0x7fbc19fb4bc8]
STDERR: 25  0x7fbc1a2060c1 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::LineWidth::updateAvailableWidth(WebCore::LayoutUnit)+0x43) [0x7fbc1a2060c1]
STDERR: 26  0x7fbc1a205fca /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::LineWidth::LineWidth(WebCore::RenderBlockFlow&, bool, WebCore::IndentTextOrNot)+0xd2) [0x7fbc1a205fca]
STDERR: 27  0x7fbc1a1f9bc4 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::LineBreaker::nextLineBreak(WebCore::BidiResolverWithIsolate<WebCore::InlineIterator, WebCore::BidiRun, WebCore::BidiIsolatedRun>&, WebCore::LineInfo&, WebCore::LineLayoutState&, WebCore::RenderTextInfo&, WebCore::FloatingObject*, unsigned int, WTF::Vector<WebCore::WordMeasurement, 64ul, WTF::CrashOnOverflow, 16ul>&)+0x12c) [0x7fbc1a1f9bc4]
STDERR: 28  0x7fbc19ff6d31 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderBlockFlow::layoutRunsAndFloatsInRange(WebCore::LineLayoutState&, WebCore::BidiResolverWithIsolate<WebCore::InlineIterator, WebCore::BidiRun, WebCore::BidiIsolatedRun>&, WebCore::InlineIterator const&, WebCore::BidiStatus const&, unsigned int)+0x369) [0x7fbc19ff6d31]
STDERR: 29  0x7fbc19ff6943 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderBlockFlow::layoutRunsAndFloats(WebCore::LineLayoutState&, bool)+0x41f) [0x7fbc19ff6943]
STDERR: 30  0x7fbc19ff90ff /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderBlockFlow::layoutLineBoxes(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&)+0x697) [0x7fbc19ff90ff]
STDERR: 31  0x7fbc19fd7395 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebCore::RenderBlockFlow::layoutInlineChildren(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&)+0xb7) [0x7fbc19fd7395]
STDERR: LEAK: 1 WebProcessPool
STDERR: LEAK: 1 WebPageProxy

[Carlos Garcia Campos]

Created attachment 277613 [details]
Patch

[Darin Adler]

Comment on attachment 277613 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277613&action=review

> Source/WebCore/platform/graphics/FontCache.cpp:326
> +        if (!data->isOpenType()) {
> +            fontVerticalDataCache().remove(addResult.iterator);
> +            return nullptr;
> +        }

This isn’t good, because it will not cache the negative result and so if OpenTypeVerticalData::create returns something that is not OpenType then it will call OpenTypeVerticalData::create again and again every time this function is called. And it will also add/remove an element to/from the hash map every time.

I think what we want instead is:

    if (!data->isOpenType())
        data = nullptr;

We will cache a nullptr and not call OpenTypeVerticalData::create again next time.

Separately, I also think it’s peculiar that OpenTypeVerticalData::create can create something that returns false to isOpenType. There’s some kind of naming mistake there. I would think, given its name, that function would return nullptr if the data is "not OpenType".

[Carlos Garcia Campos]

(In reply to comment #2)
> Comment on attachment 277613 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=277613&action=review
> 
> > Source/WebCore/platform/graphics/FontCache.cpp:326
> > +        if (!data->isOpenType()) {
> > +            fontVerticalDataCache().remove(addResult.iterator);
> > +            return nullptr;
> > +        }
> 
> This isn’t good, because it will not cache the negative result and so if
> OpenTypeVerticalData::create returns something that is not OpenType then it
> will call OpenTypeVerticalData::create again and again every time this
> function is called. And it will also add/remove an element to/from the hash
> map every time.
> 
> I think what we want instead is:
> 
>     if (!data->isOpenType())
>         data = nullptr;
> 
> We will cache a nullptr and not call OpenTypeVerticalData::create again next
> time.

Good point!

> Separately, I also think it’s peculiar that OpenTypeVerticalData::create can
> create something that returns false to isOpenType. There’s some kind of
> naming mistake there. I would think, given its name, that function would
> return nullptr if the data is "not OpenType".

Yes, I was also surprised, maybe we could move the initialization to the create method and simply return nullptr . I'll look at it in more detail.

[Carlos Garcia Campos]

Committed r200237: <http://trac.webkit.org/changeset/200237>

[Carlos Garcia Campos]

(In reply to comment #3)
> > Separately, I also think it’s peculiar that OpenTypeVerticalData::create can
> > create something that returns false to isOpenType. There’s some kind of
> > naming mistake there. I would think, given its name, that function would
> > return nullptr if the data is "not OpenType".
> 
> Yes, I was also surprised, maybe we could move the initialization to the
> create method and simply return nullptr . I'll look at it in more detail.

See https://bugs.webkit.org/show_bug.cgi?id=157172