Closed
Bug 884057
Opened 11 years ago
Closed 11 years ago
On GLX, the pixmaps used for offscreen GL contexts are leaked (regression from surfacestream big patch)
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: bjacob, Assigned: bjacob)
References
Details
(Keywords: regression, Whiteboard: [MemShrink:P2][qa-])
Attachments
(1 file)
1.21 KB,
patch
|
jgilbert
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The big patch in bug 716859 had a typo that escaped all of its reviewers: the |deleteDrawable| parameter passed by CreateOffscreenPixmapContext to CreateGLContext was accidentally changed from true to false, causing these pixmaps to be leaked. This was discovered by some valgrinding while working on bug 874768. Tested, works, green on tbpl.
Attachment #763808 -
Flags: review?(jgilbert)
Assignee | ||
Updated•11 years ago
|
Whiteboard: [MemShrink]
Comment 1•11 years ago
|
||
Do you happen to have a Valgrind log of what this leak looks like? That would help me understand if we're seeing this leak elsewhere (and catching it via DMD).
Assignee | ||
Comment 2•11 years ago
|
||
"Elsewhere"? pixmaps are a X-specific concept. Argh, I had a valgrind.log, until I overwrote it yesterday night with another one... I still have a DMD log for you: http://people.mozilla.org/~bjacob/dmd-processed.log.gz The reports look like this: Unreported: 1 block in stack trace record 6 of 5,354 9,457,664 bytes (9,453,584 requested / 4,080 slop) 1.60% of the heap (36.52% cumulative); 1.70% of unreported (38.73% cumulative) Allocated at replace_malloc (/hack/mozilla-graphics/memory/replace/dmd/DMD.cpp:1225) 0x7fa347cb2fbc malloc (/hack/mozilla-graphics/memory/build/replace_malloc.c:152) 0x41b289 _swrast_CreateContext (/usr/lib/x86_64-linux-gnu/dri/libdricore.so) 0x7fa31fa86172 intelInitContext (/usr/lib/x86_64-linux-gnu/dri/i965_dri.so) 0x7fa31fda1d3c brwCreateContext (/usr/lib/x86_64-linux-gnu/dri/i965_dri.so) 0x7fa31fdb8455 ??? (/usr/lib/x86_64-linux-gnu/dri/i965_dri.so) 0x7fa31fdaa23d ??? (/usr/lib/x86_64-linux-gnu/dri/i965_dri.so) 0x7fa31fdefd65 ??? (/usr/lib/x86_64-linux-gnu/dri/i965_dri.so) 0x7fa31fdefe7d ??? (/usr/lib/x86_64-linux-gnu/mesa/libGL.so.1) 0x7fa320ae4bd3 ??? (/usr/lib/x86_64-linux-gnu/mesa/libGL.so.1) 0x7fa320abe33e glXCreateNewContext (/usr/lib/x86_64-linux-gnu/mesa/libGL.so.1) 0x7fa320abe56a mozilla::gl::GLXLibrary::xCreateNewContext(_XDisplay*, __GLXFBConfigRec*, int, __GLXcontextRec*, int) (/hack/mozilla-graphics/gfx/gl/GLContextProvide rGLX.cpp:591) 0x7fa3434624e8 mozilla::gl::GLContextGLX::CreateGLContext(mozilla::gfx::SurfaceCaps const&, mozilla::gl::GLContextGLX*, bool, _XDisplay*, unsigned long, __GLXFBConf igRec*, bool, mozilla::gl::GLXLibrary::LibraryType, gfxXlibSurface*) (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:801) 0x7fa343462f83 CreateOffscreenPixmapContext (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:1392) 0x7fa343464aac mozilla::gl::GLContextProviderGLX::GetGlobalContext(mozilla::gl::GLContext::ContextFlags) (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:1452) 0x7fa343464d0e GetGlobalContextGLX (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:1156) 0x7fa343463f5d mozilla::gl::GLContextProviderGLX::CreateForWindow(nsIWidget*) (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:1264) 0x7fa34346440c mozilla::layers::LayerManagerOGL::CreateContext() (/hack/mozilla-graphics/gfx/layers/opengl/LayerManagerOGL.cpp:225) 0x7fa34342ab66 mozilla::layers::LayerManagerOGL::Initialize(bool) (/hack/mozilla-graphics/gfx/layers/opengl/LayerManagerOGL.cpp:264) 0x7fa34342ad9d nsBaseWidget::GetLayerManager(mozilla::layers::PLayerTransactionChild*, mozilla::layers::LayersBackend, nsIWidget::LayerManagerPersistence, bool*) (/hack/mozilla-graphics/widget/xpwidgets/nsBaseWidget.cpp:967) 0x7fa342b2350a nsWindow::GetLayerManager(mozilla::layers::PLayerTransactionChild*, mozilla::layers::LayersBackend, nsIWidget::LayerManagerPersistence, bool*) (/hack/mozilla-graphics/widget/gtk2/nsWindow.cpp:6179) 0x7fa342afe6f3 nsIWidget::GetLayerManager(bool*) (/hack/mozilla-graphics/obj-firefox-debug-dmd/embedding/browser/webBrowser/../../../dist/include/nsIWidget.h:1142) 0x7fa3417cfd60 PresShell::GetLayerManager() (/hack/mozilla-graphics/layout/base/nsPresShell.cpp:5029) 0x7fa34183c95b nsRefreshDriver::Tick(long, mozilla::TimeStamp) (/hack/mozilla-graphics/layout/base/nsRefreshDriver.cpp:1217) 0x7fa341853701
Assignee | ||
Comment 3•11 years ago
|
||
Oops, sorry, the report I showed in comment 2 is the wrong one (we fixed that leak in a different bug). The correct one is: Unreported: ~267 blocks in stack trace record 59 of 5,354 ~1,092,831 bytes (~1,092,831 requested / ~0 slop) 0.19% of the heap (60.40% cumulative); 0.20% of unreported (64.06% cumulative) Allocated at replace_calloc (/hack/mozilla-graphics/memory/replace/dmd/DMD.cpp:1244) 0x7fa347cb3067 calloc (/hack/mozilla-graphics/memory/build/replace_malloc.c:182) 0x41b3be ??? (/usr/lib/x86_64-linux-gnu/dri/i965_dri.so) 0x7fa31fdaa04f ??? (/usr/lib/x86_64-linux-gnu/dri/i965_dri.so) 0x7fa31fdeff14 ??? (/usr/lib/x86_64-linux-gnu/mesa/libGL.so.1) 0x7fa320ae54d4 ??? (/usr/lib/x86_64-linux-gnu/mesa/libGL.so.1) 0x7fa320adf524 ??? (/usr/lib/x86_64-linux-gnu/mesa/libGL.so.1) 0x7fa320adf8fa mozilla::gl::GLXLibrary::xCreatePixmap(_XDisplay*, __GLXFBConfigRec*, unsigned long, int const*) (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:655) 0x7fa3434628ac CreateOffscreenPixmapContext (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:1358) 0x7fa343464932 mozilla::gl::GLContextProviderGLX::CreateOffscreen(nsIntSize const&, mozilla::gfx::SurfaceCaps const&, mozilla::gl::GLContext::ContextFlags) (/hack/mozilla-graphics/gfx/gl/GLContextProviderGLX.cpp:1408) 0x7fa343464b74 mozilla::dom::CanvasRenderingContext2D::EnsureTarget() (/hack/mozilla-graphics/content/canvas/src/CanvasRenderingContext2D.cpp:808) 0x7fa341cc36e7 mozilla::dom::CanvasRenderingContext2D::FillRect(double, double, double, double) (/hack/mozilla-graphics/content/canvas/src/CanvasRenderingContext2D.cpp:1577) 0x7fa341cc6435 fillRect (/hack/mozilla-graphics/obj-firefox-debug-dmd/dom/bindings/CanvasRenderingContext2DBinding.cpp:1644) 0x7fa342e6f7a7 genericMethod (/hack/mozilla-graphics/obj-firefox-debug-dmd/dom/bindings/CanvasRenderingContext2DBinding.cpp:4021) 0x7fa342e77806 js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (/hack/mozilla-graphics/js/src/jscntxtinlines.h:349) 0x7fa343fadb12 js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (/hack/mozilla-graphics/js/src/vm/Interpreter.cpp:388) 0x7fa343fba29f js::Interpret(JSContext*, js::StackFrame*, js::InterpMode, bool) (/hack/mozilla-graphics/js/src/vm/Interpreter.cpp:2212) 0x7fa343fc19f4 js::RunScript(JSContext*, js::StackFrame*) (/hack/mozilla-graphics/js/src/vm/Interpreter.cpp:345) 0x7fa343fb9e5d js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (/hack/mozilla-graphics/js/src/vm/Interpreter.cpp:401) 0x7fa343fba364 js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) (/hack/mozilla-graphics/js/src/vm/Interpreter.cpp:434) 0x7fa343fba657 JS_CallFunctionValue(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::Value*) (/hack/mozilla-graphics/js/src/jsapi.cpp:5885) 0x7fa3440a51c7 mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JSObject*>, nsDOMEvent&, mozilla::ErrorResult&) (/hack/mozilla-graphics/obj-firefox-debug-dmd/dom/bindings/EventHandlerBinding.cpp:40) 0x7fa342ef3dc3 JS::Value mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, nsDOMEvent&, mozilla::ErrorResult&, mozilla::dom::CallbackObject::ExceptionHandling) (/hack/mozilla-graphics/obj-firefox-debug-dmd/dom/src/events/../../../dist/include/mozilla/dom/EventHandlerBinding.h:58) 0x7fa3421a65bb nsJSEventListener::HandleEvent(nsIDOMEvent*) (/hack/mozilla-graphics/dom/src/events/nsJSEventListener.cpp:247) 0x7fa3421a5a46
Comment 4•11 years ago
|
||
Thanks!
Updated•11 years ago
|
Attachment #763808 -
Flags: review?(jgilbert) → review+
Comment 5•11 years ago
|
||
http://jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html
Updated•11 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee | ||
Comment 6•11 years ago
|
||
Justin: yes, bool args are evil. https://hg.mozilla.org/integration/mozilla-inbound/rev/7431d7b9751f
Assignee: nobody → bjacob
Target Milestone: --- → mozilla25
Assignee | ||
Comment 7•11 years ago
|
||
Comment on attachment 763808 [details] [diff] [review] revert to destroying our pixmaps [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 716859 regressed it User impact if declined: Memory leak on desktop Linux when using WebGL (after destroying a WebGL context, some X11 resource is leaked) Testing completed (on m-c, etc.): m-i Risk to taking this patch (and alternatives if risky): very low risk -- just reverting a boolean change that was traced back to an accident/typo in bug 716859. String or IDL/UUID changes made by this patch: none
Attachment #763808 -
Flags: approval-mozilla-beta?
Attachment #763808 -
Flags: approval-mozilla-aurora?
Comment 8•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7431d7b9751f
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-firefox22:
--- → affected
status-firefox23:
--- → affected
status-firefox24:
--- → affected
status-firefox25:
--- → fixed
Keywords: regression
Comment 9•11 years ago
|
||
Comment on attachment 763808 [details] [diff] [review] revert to destroying our pixmaps Early enough in the cycle to take a fix for this FF22 regression.
Attachment #763808 -
Flags: approval-mozilla-beta?
Attachment #763808 -
Flags: approval-mozilla-beta+
Attachment #763808 -
Flags: approval-mozilla-aurora?
Attachment #763808 -
Flags: approval-mozilla-aurora+
Comment 10•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/4fbeaf9b6c76 https://hg.mozilla.org/releases/mozilla-beta/rev/384a7ae08ab1
Comment 11•11 years ago
|
||
De-prioritizing QA verification of this fix since it's very low risk. Please remove the [qa-] tag and add the verifyme keyword if this needs prioritization.
Whiteboard: [MemShrink:P2] → [MemShrink:P2][qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•