webkit_unittests CSSFontFaceSourceTest.UnboundedGrowth fails when compiled with MSVC |
|||
Issue descriptionFrom https://ci.chromium.org/buildbot/chromium.clang/CrWinClang%28shared%29/1540 [ RUN ] CSSFontFaceSourceTest.UnboundedGrowth [588:4528:0214/153951.025:2538822:FATAL:HashTable.h(600)] Security DCHECK failed: container_modifications_ == container_->Modifications(). Backtrace: base::debug::StackTrace::StackTrace [0x7415BD47+55] base::debug::StackTrace::StackTrace [0x7415B89A+10] WTF::HashTableAddResult<WTF::HashTable<blink::FontCacheKey,WTF::KeyValuePair<blink::FontCacheKey,scoped_refptr<blink::SimpleFontData> >,WTF::KeyValuePairKeyExtractor,blink::FontCacheKeyHash,WTF::HashMapValueTraits<blink::FontCacheKeyTraits,WTF::HashTraits [0x6E5DA655+109] blink::CSSFontFaceSource::GetFontData [0x6E5DAFC4+312] blink::CSSFontFaceSourceTest_UnboundedGrowth_Test::TestBody [0x017C0BF1+401] testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void> [0x012D7099+32] testing::Test::Run [0x012E3CCC+93] testing::TestCase::Run [0x012E3D99+133] testing::internal::UnitTestImpl::RunAllTests [0x012E41D2+466] testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool> [0x012D7117+32] testing::UnitTest::Run [0x012E3FAF+136] base::TestSuite::Run [0x02388A16+102] ?Run@?$Invoker@U?$BindState@P6AXXZ$$V@internal@base@@$$A6AXXZ@internal@base@@SAXPAVBindStateBase@23@@Z [0x0121BABB+43] base::internal::Invoker<base::internal::BindState<int (__cdecl*)(base::TestSuite *),base::internal::UnretainedWrapper<base::TestSuite> >,int __cdecl(void)>::Run [0x0121BA7E+14] base::LaunchUnitTests [0x023859D7+813] base::LaunchUnitTests [0x0238567B+86] main [0x0121BBE3+75] __scrt_common_main_seh [0x0259628A+248] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283) BaseThreadInitThunk [0x76E3338A+18] RtlInitializeExceptionChain [0x77939902+99] RtlInitializeExceptionChain [0x779398D5+54] Other builders: https://ci.chromium.org/buildbot/chromium.clang/CrWinClang%28dbg%29/1463 https://ci.chromium.org/buildbot/chromium.clang/CrWinClang64%28dbg%29/897 https://ci.chromium.org/buildbot/chromium.clang/CrWinClang64%28dll%29/841 It doesn't seem to repro on the release builders, maybe because those don't enable the dcheck that's getting hit?
,
Feb 15 2018
I think the test failed when it landed: https://chromium.googlesource.com/chromium/src/+/0d4a27248ff0b9aace5be15c041cd21e9a0443fa
,
Feb 15 2018
Verified locally that the test has been failing in this config since it landed.
,
Feb 15 2018
The code looks like this:
scoped_refptr<SimpleFontData> CSSFontFaceSource::GetFontData(
const FontDescription& font_description,
const FontSelectionCapabilities& font_selection_capabilities) {
// If the font hasn't loaded or an error occurred, then we've got nothing.
if (!IsValid())
return nullptr;
if (IsLocal()) {
// We're local. Just return a SimpleFontData from the normal cache.
return CreateFontData(font_description, font_selection_capabilities);
}
FontCacheKey key = font_description.CacheKey(FontFaceCreationParams());
scoped_refptr<SimpleFontData>& font_data =
font_data_table_.insert(key, nullptr).stored_value->value; // << ----
if (!font_data)
font_data = CreateFontData(font_description, font_selection_capabilities);
font_cache_key_age.PrependOrMoveToFirst(key);
PruneOldestIfNeeded();
DCHECK_LE(font_data_table_.size(), kMaxCachedFontData);
// No release, because fontData is a reference to a RefPtr that is held in the
// font_data_table_.
return font_data;
}
I think what's happening is that MSVC extends the lifetime of the object returned by font_data_table_.insert() until the end of the function. When that object, a WTF::HashTableAddResult, destructs after the PruneOldestIfNeeded(), it trips the dcheck.
,
Feb 15 2018
A potential fix: https://chromium-review.googlesource.com/#/c/chromium/src/+/921942
,
Feb 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ce5486ae573d41c2d29e1e58308b99355595b0be commit ce5486ae573d41c2d29e1e58308b99355595b0be Author: Hans Wennborg <hans@chromium.org> Date: Thu Feb 15 18:43:23 2018 Fix CSSFontFaceSourceTest.UnboundedGrowth test in MSVC builds The WTF::HashTableAddResult from calling font_data_table_.insert() was being lifetime-extended in MSVC builds to the end of the function, causing the HashTableAddResult's destructor to run after the hash table had been changed, causing a dcheck. Bug: 812600 Change-Id: Ifa2b05ef92370c7ba043eea4258f4086274d1871 Reviewed-on: https://chromium-review.googlesource.com/921942 Reviewed-by: Dominik Röttsches <drott@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#537085} [modify] https://crrev.com/ce5486ae573d41c2d29e1e58308b99355595b0be/third_party/WebKit/Source/core/css/CSSFontFaceSource.cpp
,
Feb 15 2018
,
Feb 16 2018
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d93525cde8ce7151e584db7bd690a09c06b83715 commit d93525cde8ce7151e584db7bd690a09c06b83715 Author: Reid Kleckner <rnk@google.com> Date: Thu Mar 01 01:26:11 2018 Fix dangling reference into font data hashtable It's unlikely that pruning the cache will resize the hash table, but this makes a copy of the scoped_refptr just in case. R=drott@chromium.org BUG= 812600 Change-Id: I5a6256b9079e9c7f36249cadbb679189f0248afd Reviewed-on: https://chromium-review.googlesource.com/852531 Reviewed-by: Hans Wennborg <hans@chromium.org> Reviewed-by: Dominik Röttsches <drott@chromium.org> Commit-Queue: Reid Kleckner <rnk@chromium.org> Cr-Commit-Position: refs/heads/master@{#539970} [modify] https://crrev.com/d93525cde8ce7151e584db7bd690a09c06b83715/third_party/WebKit/Source/core/css/CSSFontFaceSource.cpp |
|||
►
Sign in to add a comment |
|||
Comment 1 by h...@chromium.org
, Feb 15 2018