1633112 - Crash in [@ GMut::EnsureInUse] through [@ mozilla::layers::WebRenderBridgeParent::AddPendingScrollPayload] with use-after-free

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[Christian Holler (:decoder)]

This bug is for crash report bp-98669b45-687a-4f10-b1f6-239970200306.

Top 10 frames of crashing thread:

0 mozglue.dll GMut::EnsureInUse memory/replace/phc/PHC.cpp:683
1 mozglue.dll replace_realloc memory/replace/phc/PHC.cpp:1117
2 mozglue.dll moz_xrealloc memory/mozalloc/mozalloc.cpp:72
3 xul.dll nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator> xpcom/ds/nsTArray-inl.h:191
4 xul.dll mozilla::layers::WebRenderBridgeParent::AddPendingScrollPayload gfx/layers/wr/WebRenderBridgeParent.cpp:1055
5 xul.dll mozilla::layers::APZCTreeManager::SampleForWebRender gfx/layers/apz/src/APZCTreeManager.cpp:722
6 xul.dll mozilla::layers::APZSampler::SampleForWebRender gfx/layers/apz/src/APZSampler.cpp:99
7 xul.dll static mozilla::layers::APZSampler::SampleForWebRender gfx/layers/apz/src/APZSampler.cpp:74
8 xul.dll apz_sample_transforms gfx/layers/apz/src/APZSampler.cpp:276
9 xul.dll webrender_bindings::bindings::{{impl}}::sample gfx/webrender_bindings/src/bindings.rs:975

PHC Free/Alloc Stacks:

Free stack:

#0    PLDHashTable::RemoveEntry(PLDHashEntryHdr*) (xul.pdb)
#1    mozilla::layers::WebRenderBridgeParent::RemovePendingScrollPayload(std::pair<mozilla::wr::PipelineId,mozilla::wr::Epoch> const&) (xul.pdb)
#2    mozilla::layers::CompositorBridgeParent::NotifyPipelineRendered(mozilla::wr::PipelineId const&, mozilla::wr::Epoch const&, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType> const&, mozilla::TimeStamp&, mozilla::TimeStamp&, mozilla::TimeStamp&, mozilla::wr::RendererStats*) (xul.pdb)
#3    mozilla::wr::NotifyDidRender(mozilla::layers::CompositorBridgeParent*, RefPtr<const mozilla::wr::WebRenderPipelineInfo>, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, mozilla::TimeStamp, mozilla::TimeStamp, bool, mozilla::wr::RendererStats) (xul.pdb)
#4    RunnableFunction<void (*)(mozilla::layers::CompositorBridgeParent *, RefPtr<const mozilla::wr::WebRenderPipelineInfo>, mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, mozilla::TimeStamp, mozilla::TimeStamp, bool, mozilla::wr::RendererStats),mozilla::Tuple<mozilla::layers::CompositorBridgeParent *,RefPtr<const mozilla::wr::WebRenderPipelineInfo>,mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>,mozilla::TimeStamp,mozilla::TimeStamp,mozilla::TimeStamp,bool,mozilla::wr::RendererStats> >::Run() (xul.pdb)
#5    MessageLoop::DoWork() (xul.pdb)
#6    base::MessagePumpForUI::DoRunLoop() (xul.pdb)
#7    base::MessagePumpWin::Run(base::MessagePump::Delegate*) (xul.pdb)
#8    MessageLoop::RunHandler() (xul.pdb)
#9    MessageLoop::Run() (xul.pdb)
#10    base::Thread::ThreadMain() (xul.pdb)
#11    `anonymous namespace'::ThreadFunc(void*) (xul.pdb)
#12    BaseThreadInitThunk (kernel32.pdb)
#13    patched_BaseThreadInitThunk(int, void*, void*) (mozglue.pdb)
#14    RtlUserThreadStart (ntdll.pdb)

Alloc stack:

#0    nsTArray_base<nsTArrayInfallibleAllocator,nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>(unsigned long long, unsigned long long) (xul.pdb)
#1    mozilla::layers::WebRenderBridgeParent::AddPendingScrollPayload(mozilla::layers::CompositionPayload&, std::pair<mozilla::wr::PipelineId,mozilla::wr::Epoch> const&) (xul.pdb)
#2    mozilla::layers::APZCTreeManager::SampleForWebRender(mozilla::wr::TransactionWrapper&, mozilla::TimeStamp const&, mozilla::wr::RenderRoot, nsTArray<mozilla::wr::WrPipelineIdAndEpoch> const*) (xul.pdb)
#3    mozilla::layers::APZSampler::SampleForWebRender(mozilla::wr::TransactionWrapper&, mozilla::wr::RenderRoot, nsTArray<mozilla::wr::WrPipelineIdAndEpoch> const*) (xul.pdb)
#4    static mozilla::layers::APZSampler::SampleForWebRender(mozilla::wr::WrWindowId const&, mozilla::wr::Transaction*, mozilla::wr::DocumentId const&, nsTArray<mozilla::wr::WrPipelineIdAndEpoch> const*) (xul.pdb)
#5    apz_sample_transforms(mozilla::wr::WrWindowId, mozilla::wr::Transaction*, mozilla::wr::DocumentId, nsTArray<mozilla::wr::WrPipelineIdAndEpoch> const*) (xul.pdb)
#6    webrender_bindings::bindings::{{impl}}::sample(webrender_bindings::bindings::SamplerCallback*, webrender_api::api::DocumentId, std::collections::hash::map::HashMap<webrender_api::api::PipelineId, webrender_api::api::Epoch, core::hash::BuildHasherDefault<fxhash::FxHasher>>*) (xul.pdb)
#7    webrender::render_backend::RenderBackend::update_document(webrender_api::api::DocumentId, alloc::vec::Vec<webrender_api::api::ResourceUpdate>, core::option::Option<webrender::scene_builder_thread::InternerUpdates>, alloc::vec::Vec<webrender_api::api::FrameMsg>, alloc::vec::Vec<webrender_api::api::NotificationRequest>, bool, bool, unsigned int*, webrender::profiler::BackendProfileCounters*, bool) (xul.pdb)
#8    webrender::render_backend::RenderBackend::run(webrender::profiler::BackendProfileCounters) (xul.pdb)
#9    std::sys_common::backtrace::__rust_begin_short_backtrace<closure-4,()>(webrender::renderer::{{impl}}::new::closure-4) (xul.pdb)
#10    core::ops::function::FnOnce::call_once<closure-0,()>(std::thread::{{impl}}::spawn_unchecked::closure-0*) (xul.pdb)
#11    alloc::boxed::{{impl}}::call_once<(),FnOnce<()>>() (xul.pdb)
#12    std::sys::windows::thread::{{impl}}::new::thread_start() (xul.pdb)
#13    BaseThreadInitThunk (kernel32.pdb)
#14    patched_BaseThreadInitThunk(int, void*, void*) (mozglue.pdb)
#15    RtlUserThreadStart (ntdll.pdb)

This is an older report from March that I only saw now but it might still be valid. If necessary, we can symbolize the alloc/free traces locally to get line numbers.

Comment 1

[Sean Feng [:sefeng]]

I don't think we need to worry about this.

There was a bug in https://phabricator.services.mozilla.com/D60046 that two threads accessing mPendingScrollPayloads in the same time, one adding and one removing for the same key, and since it wasn't thread safe, it caused this use after free.

The patch got landed (Feb 21) backed out (Feb 24), and we fixed and relanded (March 12). This crash build was 20200223214228, so it used the older version of the patch. And based on the graph, there are no further crashes. I think we are good here.

Comment 2

[Christian Holler (:decoder)]

Fixed by the backout/reland in bug 1600793.

Thanks for double-checking!