Bug 223790

Summary: ASSERTION FAILED: m_clients.contains(&client) in CSSFontFace::removeClient via CSSSegmentedFontFace::~CSSSegmentedFontFace()
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: CSSAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, cgarcia, ews-feeder, fred.wang, gpoo, koivisto, mmaxfield, product-security, rbuis, sabouhallawa, svillar, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Test
none
Reduced testcase
none
Super reduced test case
none
Patch none

Description Ryosuke Niwa 2021-03-26 03:03:04 PDT 
Created attachment 424340 [details]
Test

e.g.

ASSERTION FAILED: m_clients.contains(&client)
./css/CSSFontFace.cpp(404) : void WebCore::CSSFontFace::removeClient(WebCore::CSSFontFace::Client &)
1   0x538de4cc9 WTFCrash
2   0x5186c550b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x51b2a5cdb WebCore::CSSFontFace::removeClient(WebCore::CSSFontFace::Client&)
4   0x51b33c029 WebCore::CSSSegmentedFontFace::~CSSSegmentedFontFace()
5   0x51b33c105 WebCore::CSSSegmentedFontFace::~CSSSegmentedFontFace()
6   0x51b2cb38b std::__1::default_delete<WebCore::CSSSegmentedFontFace>::operator()(WebCore::CSSSegmentedFontFace*) const
7   0x51b2cb352 WTF::RefCounted<WebCore::CSSSegmentedFontFace, std::__1::default_delete<WebCore::CSSSegmentedFontFace> >::deref() const
8   0x51b2cb2fe WebCore::CSSSegmentedFontFace::deref()
...
24  0x51b2bbd3a WebCore::CSSFontFaceSet::remove(WebCore::CSSFontFace const&)
25  0x51b2c3bd7 WebCore::CSSFontSelector::addFontFaceRule(WebCore::StyleRuleFontFace&, bool)
26  0x51d0d9893 WebCore::Style::Resolver::addCurrentSVGFontFaceRules()
27  0x51d0ed990 WebCore::Style::Scope::resolver()
28  0x51d0f4e4a WebCore::Style::TreeResolver::Scope::Scope(WebCore::Document&)
29  0x51d0f4ebd WebCore::Style::TreeResolver::Scope::Scope(WebCore::Document&)
30  0x51d0f8635 WebCore::Style::TreeResolver::resolve()
31  0x51b5bc23d WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType)

<rdar://75879757>
Comment 1 Frédéric Wang (:fredw) 2021-03-30 05:44:11 PDT 
Created attachment 424631 [details]
Reduced testcase
Comment 2 Sergio Villar Senin 2021-05-05 09:15:46 PDT 
Created attachment 427772 [details]
Super reduced test case
Comment 3 Sergio Villar Senin 2021-05-06 12:07:56 PDT 
Created attachment 427917 [details]
Patch
Comment 4 Sergio Villar Senin 2021-05-06 12:13:41 PDT 
I'm attaching a test case because I think this is not a security issue. We're just trying to remove something from a HashSet twice.

In the proposed patch I decided to add some code to the loop that calls appendFontFace() in CSSFontFaceSet::fontFace(). A couple of comments:

* Removing duplicate entries could be done with removeRepeatedElements() too but that would mean an extra unneeded traversal of the Vector.
* My first thought was using a ListHashSet for candidateFontFaces instead of a Vector to avoid the duplicates. However that is not possible because we don't have the proper operators to be able to run std::stable_sort
Comment 5 EWS 2021-05-12 10:58:26 PDT 
Committed r277378 (237634@main): <https://commits.webkit.org/237634@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427917 [details].