574585 - LayerManagerOGL should release its root layer before destruction

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Otherwise we get the crash below.  "Blocked" on bug 564991 in that it's not strictly needed until after layer retaining, but this patch can land independently.

=27630== Invalid read of size 8
==27630==    at 0x6DC6E96: mozilla::layers::LayerManagerOGL::MakeCurrent() (LayerManagerOGL.cpp:353)
==27630==    by 0x6DCA8BC: mozilla::layers::ThebesLayerOGL::~ThebesLayerOGL() (ThebesLayerOGL.cpp:87)
==27630==    by 0x58E3FD4: mozilla::layers::Layer::Release() (Layers.h:308)
==27630==    by 0x6DC1576: mozilla::layers::ContainerLayerOGL::RemoveChild(mozilla::layers::Layer*) (ContainerLayerOGL.cpp:100)
==27630==    by 0x6DC12E7: mozilla::layers::ContainerLayerOGL::~ContainerLayerOGL() (ContainerLayerOGL.cpp:53)
==27630==    by 0x58E3FD4: mozilla::layers::Layer::Release() (Layers.h:308)
==27630==    by 0x6DC1576: mozilla::layers::ContainerLayerOGL::RemoveChild(mozilla::layers::Layer*) (ContainerLayerOGL.cpp:100)
==27630==    by 0x6DC12E7: mozilla::layers::ContainerLayerOGL::~ContainerLayerOGL() (ContainerLayerOGL.cpp:53)
==27630==    by 0x58E3FD4: mozilla::layers::Layer::Release() (Layers.h:308)
==27630==    by 0x58E4408: nsRefPtr<mozilla::layers::Layer>::~nsRefPtr() (nsAutoPtr.h:969)
==27630==    by 0x6DBC967: mozilla::layers::LayerManager::~LayerManager() (Layers.h:154)
==27630==    by 0x6DC5811: mozilla::layers::LayerManagerOGL::~LayerManagerOGL() (LayerManagerOGL.cpp:86)
==27630==    by 0x58E3D9A: mozilla::layers::LayerManager::Release() (Layers.h:139)
==27630==    by 0x58E4488: nsRefPtr<mozilla::layers::LayerManager>::~nsRefPtr() (nsAutoPtr.h:969)
==27630==    by 0x58DE537: mozilla::FrameLayerBuilder::InternalDestroyDisplayItemData(nsIFrame*, void*, int) (FrameLayerBuilder.cpp:296)
==27630==    by 0x58DE567: mozilla::FrameLayerBuilder::DestroyDisplayItemData(nsIFrame*, void*) (FrameLayerBuilder.cpp:303)
==27630==    by 0x58E7EC2: mozilla::FramePropertyTable::PropertyValue::DestroyValueFor(nsIFrame*) (FramePropertyTable.h:178)
==27630==    by 0x58E7C32: mozilla::FramePropertyTable::DeleteAllForEntry(mozilla::FramePropertyTable::Entry*) (FramePropertyTable.cpp:214)
==27630==    by 0x58E7D69: mozilla::FramePropertyTable::DeleteEnumerator(mozilla::FramePropertyTable::Entry*, void*) (FramePropertyTable.cpp:248)
==27630==    by 0x58E8467: nsTHashtable<mozilla::FramePropertyTable::Entry>::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) (nsTHashtable.h:420)
==27630==    by 0x6BEFD79: PL_DHashTableEnumerate (pldhash.c:754)
==27630==    by 0x58E8236: nsTHashtable<mozilla::FramePropertyTable::Entry>::EnumerateEntries(PLDHashOperator (*)(mozilla::FramePropertyTable::Entry*, void*), void*) (nsTHashtable.h:241)
==27630==    by 0x58E7DAD: mozilla::FramePropertyTable::DeleteAll() (FramePropertyTable.cpp:258)
==27630==    by 0x597534A: PresShell::Destroy() (nsPresShell.cpp:1949)
==27630==    by 0x5943F9A: DocumentViewerImpl::DestroyPresShell() (nsDocumentViewer.cpp:4234)
==27630==    by 0x5939E07: DocumentViewerImpl::Destroy() (nsDocumentViewer.cpp:1621)
==27630==    by 0x65E11EE: nsDocShell::Destroy() (nsDocShell.cpp:4345)
==27630==    by 0x5C98862: nsFrameLoader::Finalize() (nsFrameLoader.cpp:352)
==27630==    by 0x5C77684: nsDocument::MaybeInitializeFinalizeFrameLoaders() (nsDocument.cpp:5153)
==27630==    by 0x5C8D5BF: nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() (nsThreadUtils.h:347)


==27630==  Address 0x18362b30 is 0 bytes inside a block of size 1,240 free'd
==27630==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==27630==    by 0x6CA693B: moz_free (nsTraceMalloc.c:1264)
==27630==    by 0x6DB601C: mozilla::gl::GLContextGLX::~GLContextGLX() (mozalloc.h:250)
==27630==    by 0x5D4CAF7: mozilla::gl::GLContext::Release() (GLContext.h:119)
==27630==    by 0x5D4D5EE: nsRefPtr<mozilla::gl::GLContext>::~nsRefPtr() (nsAutoPtr.h:969)
==27630==    by 0x6DC57F5: mozilla::layers::LayerManagerOGL::~LayerManagerOGL() (LayerManagerOGL.cpp:86)
==27630==    by 0x58E3D9A: mozilla::layers::LayerManager::Release() (Layers.h:139)
==27630==    by 0x58E4488: nsRefPtr<mozilla::layers::LayerManager>::~nsRefPtr() (nsAutoPtr.h:969)
==27630==    by 0x58DE537: mozilla::FrameLayerBuilder::InternalDestroyDisplayItemData(nsIFrame*, void*, int) (FrameLayerBuilder.cpp:296)
==27630==    by 0x58DE567: mozilla::FrameLayerBuilder::DestroyDisplayItemData(nsIFrame*, void*) (FrameLayerBuilder.cpp:303)
==27630==    by 0x58E7EC2: mozilla::FramePropertyTable::PropertyValue::DestroyValueFor(nsIFrame*) (FramePropertyTable.h:178)
==27630==    by 0x58E7C32: mozilla::FramePropertyTable::DeleteAllForEntry(mozilla::FramePropertyTable::Entry*) (FramePropertyTable.cpp:214)
==27630==    by 0x58E7D69: mozilla::FramePropertyTable::DeleteEnumerator(mozilla::FramePropertyTable::Entry*, void*) (FramePropertyTable.cpp:248)
==27630==    by 0x58E8467: nsTHashtable<mozilla::FramePropertyTable::Entry>::s_EnumStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) (nsTHashtable.h:420)
==27630==    by 0x6BEFD79: PL_DHashTableEnumerate (pldhash.c:754)
==27630==    by 0x58E8236: nsTHashtable<mozilla::FramePropertyTable::Entry>::EnumerateEntries(PLDHashOperator (*)(mozilla::FramePropertyTable::Entry*, void*), void*) (nsTHashtable.h:241)
==27630==    by 0x58E7DAD: mozilla::FramePropertyTable::DeleteAll() (FramePropertyTable.cpp:258)
==27630==    by 0x597534A: PresShell::Destroy() (nsPresShell.cpp:1949)
==27630==    by 0x5943F9A: DocumentViewerImpl::DestroyPresShell() (nsDocumentViewer.cpp:4234)
==27630==    by 0x5939E07: DocumentViewerImpl::Destroy() (nsDocumentViewer.cpp:1621)
==27630==    by 0x65E11EE: nsDocShell::Destroy() (nsDocShell.cpp:4345)
==27630==    by 0x5C98862: nsFrameLoader::Finalize() (nsFrameLoader.cpp:352)
==27630==    by 0x5C77684: nsDocument::MaybeInitializeFinalizeFrameLoaders() (nsDocument.cpp:5153)
==27630==    by 0x5C8D5BF: nsRunnableMethodImpl<void (nsDocument::*)(), true>::Run() (nsThreadUtils.h:347)
==27630==    by 0x5C31AA1: nsContentUtils::AddScriptRunner(nsIRunnable*) (nsContentUtils.cpp:4598)
==27630==    by 0x5C7741A: nsDocument::FinalizeFrameLoader(nsFrameLoader*) (nsDocument.cpp:5109)
==27630==    by 0x5C9BA23: nsFrameLoader::Destroy() (nsFrameLoader.cpp:997)
==27630==    by 0x624BEC5: nsXULElement::DestroyContent() (nsXULElement.cpp:1572)
==27630==    by 0x5CB43D4: nsGenericElement::DestroyContent() (nsGenericElement.cpp:3779)
==27630==    by 0x624BEE6: nsXULElement::DestroyContent() (nsXULElement.cpp:1577)
==27630==

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Have LayerManagerOGL release its root layer before destruction

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/da4eaba07c92