Bug 219953 – CachedRefPtr should adoptRef before calling ref to appease RefCounted's debug assertions

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Ryan Haddad]

616 tests are asserting on the debug JSC bot

https://build.webkit.org/builders/Apple-Catalina-Debug-JSC-Tests/builds/2053/steps/jscore-test/logs/stdio

ASSERTION FAILED: !m_adoptionIsRequired
/Volumes/Data/slave/catalina-debug/build/WebKitBuild/Debug/usr/local/include/wtf/RefCounted.h(47) : void WTF::RefCountedBase::ref() const
1   0x10d832cc9 WTFCrash
2   0x10f00ef5b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x10f1dd28c WTF::RefCountedBase::ref() const
4   0x10e2fd391 WTF::DefaultRefDerefTraits<JSC::TDZEnvironmentLink>::refIfNotNull(JSC::TDZEnvironmentLink*)
5   0x10f0a4818 JSC::CachedRefPtr<JSC::CachedTDZEnvironmentLink, JSC::TDZEnvironmentLink, WTF::RawPtrTraits<JSC::TDZEnvironmentLink> >::decode(JSC::Decoder&) const
6   0x10f0a4775 JSC::CachedRefPtr<JSC::CachedTDZEnvironmentLink, JSC::TDZEnvironmentLink, WTF::RawPtrTraits<JSC::TDZEnvironmentLink> >::decode(JSC::Decoder&, WTF::RefPtr<JSC::TDZEnvironmentLink, WTF::RawPtrTraits<JSC::TDZEnvironmentLink>, WTF::DefaultRefDerefTraits<JSC::TDZEnvironmentLink> >&) const
7   0x10f0a45c1 JSC::CachedFunctionExecutableRareData::decode(JSC::Decoder&) const
8   0x10f0a435b JSC::UnlinkedFunctionExecutable::RareData* JSC::CachedPtr<JSC::CachedFunctionExecutableRareData, JSC::UnlinkedFunctionExecutable::RareData>::decode<>(JSC::Decoder&, bool&) const
9   0x10f0a4281 JSC::UnlinkedFunctionExecutable::RareData* JSC::CachedPtr<JSC::CachedFunctionExecutableRareData, JSC::UnlinkedFunctionExecutable::RareData>::decode<>(JSC::Decoder&) const
10  0x10f0a4016 JSC::CachedFunctionExecutable::rareData(JSC::Decoder&) const
11  0x10f0a3b0f JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(JSC::Decoder&, JSC::CachedFunctionExecutable const&)
12  0x10f0a3555 JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(JSC::Decoder&, JSC::CachedFunctionExecutable const&)
13  0x10f0a3417 JSC::CachedFunctionExecutable::decode(JSC::Decoder&) const
14  0x10f0a323b JSC::UnlinkedFunctionExecutable* JSC::CachedPtr<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>::decode<>(JSC::Decoder&, bool&) const
15  0x10f0a3051 JSC::UnlinkedFunctionExecutable* JSC::CachedPtr<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>::decode<>(JSC::Decoder&) const
16  0x10f0a2ff5 JSC::CachedWriteBarrier<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>::decode(JSC::Decoder&, JSC::WriteBarrier<JSC::UnlinkedFunctionExecutable, WTF::RawPtrTraits<JSC::UnlinkedFunctionExecutable> >&, JSC::JSCell const*) const
17  0x10f0a2b50 std::__1::enable_if<!(std::is_same<JSC::CachedWriteBarrier<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>, JSC::SourceTypeImpl<JSC::CachedWriteBarrier<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>, void>::type>::value), void>::type JSC::decode<JSC::CachedWriteBarrier<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>, JSC::UnlinkedCodeBlock*>(JSC::Decoder&, JSC::CachedWriteBarrier<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable> const&, JSC::SourceTypeImpl<JSC::CachedWriteBarrier<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>, void>::type&, JSC::UnlinkedCodeBlock*)
18  0x10f09c3f3 void JSC::CachedVector<JSC::CachedWriteBarrier<JSC::CachedFunctionExecutable, JSC::UnlinkedFunctionExecutable>, 0ul, WTF::CrashOnOverflow, WTF::FastMalloc>::decode<JSC::UnlinkedCodeBlock*>(JSC::Decoder&, WTF::RefCountedArray<JSC::WriteBarrier<JSC::UnlinkedFunctionExecutable, WTF::RawPtrTraits<JSC::UnlinkedFunctionExecutable> >, WTF::RawPtrTraits<JSC::WriteBarrier<JSC::UnlinkedFunctionExecutable, WTF::RawPtrTraits<JSC::UnlinkedFunctionExecutable> > > >&, JSC::UnlinkedCodeBlock*) const
19  0x10f091c0d JSC::CachedCodeBlock<JSC::UnlinkedProgramCodeBlock>::decode(JSC::Decoder&, JSC::UnlinkedCodeBlock&) const
20  0x10f091913 JSC::CachedProgramCodeBlock::decode(JSC::Decoder&) const
21  0x10f0916fb JSC::UnlinkedProgramCodeBlock* JSC::CachedPtr<JSC::CachedProgramCodeBlock, JSC::UnlinkedProgramCodeBlock>::decode<>(JSC::Decoder&, bool&) const
22  0x10f08e2d1 JSC::UnlinkedProgramCodeBlock* JSC::CachedPtr<JSC::CachedProgramCodeBlock, JSC::UnlinkedProgramCodeBlock>::decode<>(JSC::Decoder&) const
23  0x10f060479 JSC::CacheEntry<JSC::UnlinkedProgramCodeBlock>::decode(JSC::Decoder&, std::__1::pair<JSC::SourceCodeKey, JSC::UnlinkedProgramCodeBlock*>&) const
24  0x10f06020e JSC::GenericCacheEntry::decode(JSC::Decoder&, std::__1::pair<JSC::SourceCodeKey, JSC::UnlinkedCodeBlock*>&) const
25  0x10f061023 JSC::decodeCodeBlockImpl(JSC::VM&, JSC::SourceCodeKey const&, WTF::Ref<JSC::CachedBytecode, WTF::RawPtrTraits<JSC::CachedBytecode> >)
26  0x10f0bc725 JSC::UnlinkedProgramCodeBlock* JSC::decodeCodeBlock<JSC::UnlinkedProgramCodeBlock>(JSC::VM&, JSC::SourceCodeKey const&, WTF::Ref<JSC::CachedBytecode, WTF::RawPtrTraits<JSC::CachedBytecode> >)
27  0x10f0bc5ab JSC::UnlinkedProgramCodeBlock* JSC::CodeCacheMap::fetchFromDiskImpl<JSC::UnlinkedProgramCodeBlock>(JSC::VM&, JSC::SourceCodeKey const&)
28  0x10f0bc1c5 std::__1::enable_if<(std::is_base_of<JSC::UnlinkedCodeBlock, JSC::UnlinkedProgramCodeBlock>::value) && (!(std::is_same<JSC::UnlinkedProgramCodeBlock, JSC::UnlinkedEvalCodeBlock>::value)), JSC::UnlinkedProgramCodeBlock*>::type JSC::CodeCacheMap::fetchFromDisk<JSC::UnlinkedProgramCodeBlock>(JSC::VM&, JSC::SourceCodeKey const&)
29  0x10f0bbcd8 JSC::UnlinkedProgramCodeBlock* JSC::CodeCacheMap::findCacheAndUpdateAge<JSC::UnlinkedProgramCodeBlock>(JSC::VM&, JSC::SourceCodeKey const&)
30  0x10f064353 JSC::UnlinkedProgramCodeBlock* JSC::CodeCache::getUnlinkedGlobalCodeBlock<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable>(JSC::VM&, JSC::ProgramExecutable*, JSC::SourceCode const&, JSC::JSParserStrictMode, JSC::JSParserScriptMode, WTF::OptionSet<JSC::CodeGenerationMode>, JSC::ParserError&, JSC::EvalContextType)
31  0x10f0641c9 JSC::CodeCache::getUnlinkedProgramCodeBlock(JSC::VM&, JSC::ProgramExecutable*, JSC::SourceCode const&, JSC::JSParserStrictMode, WTF::OptionSet<JSC::CodeGenerationMode>, JSC::ParserError&)

[Radar WebKit Bug Importer]

<rdar://problem/72391255>

[Ryan Haddad]

Maybe https://trac.webkit.org/changeset/270870/webkit?

[Saam Barati]

Created attachment 416531 [details]
patch

[Tadeu Zagallo]

Comment on attachment 416531 [details]
patch

r=me

[Darin Adler]

Comment on attachment 416531 [details]
patch

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

> Source/JavaScriptCore/runtime/CachedTypes.cpp:560
> +        RefPtr<Source, PtrTraits> result = adoptRef(decodedPtr);

Two related questions:

1) Is it OK that this calls adoptRef without template arguments rather than calling adoptRef<Source, PtrTraits>?
2) Would you consider using auto here instead of writing out the RefPtr type?

[Saam Barati]

(In reply to Darin Adler from comment #5)
> Comment on attachment 416531 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=416531&action=review
> 
> > Source/JavaScriptCore/runtime/CachedTypes.cpp:560
> > +        RefPtr<Source, PtrTraits> result = adoptRef(decodedPtr);
> 
> Two related questions:
> 
> 1) Is it OK that this calls adoptRef without template arguments rather than
> calling adoptRef<Source, PtrTraits>?
Probably not. Good call, I will fix. This is probably fine for the particular types we currently use it for, but it'd probably not work for future types that make use of non default PtrTraits.

> 2) Would you consider using auto here instead of writing out the RefPtr type?
Auto seems ok

[Saam Barati]

Created attachment 416547 [details]
patch for landing

[EWS]

Committed r270991: <https://trac.webkit.org/changeset/270991>

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