Valgrind leak when the window is maximized

RESOLVED FIXED in Firefox 46

Status

()

Core
Widget: Gtk
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

(Blocks: 1 bug)

Trunk
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 affected, firefox46 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Attachment 8692011 [details] [diff], my patch for bug 384336, causes a permanent Valgrind leak:

https://treeherder.mozilla.org/logviewer.html#?job_id=5925985&repo=fx-team

Specifically, if I comment out these lines, the leak goes away:

>      if (width < TARGET_WIDTH && height < TARGET_HEIGHT) {
>        document.documentElement.setAttribute("sizemode", "maximized");
>      }

I don't know what's going on, but I'm guessing it's either a widget or a layout bug. Does this seem possible?
(Assignee)

Updated

2 years ago
Flags: needinfo?(karlt)
(Assignee)

Comment 1

2 years ago
(In reply to Dão Gottwald [:dao] from comment #0)
> I don't know what's going on, but I'm guessing it's either a widget or a
> layout bug.

Or graphics, but my first bet would be Gtk widget...
40 bytes in 1 blocks are definitely lost in loss record 4,763 of 10,455
   at 0x4C293D5: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x9F5CFCF: _cairo_freelist_alloc (cairo-freelist.c:64)
   by 0x9FA1ADB: _cairo_xlib_display_queue_resource (cairo-xlib-display.c:488)
   by 0x9FA7F78: _cairo_xlib_surface_finish (cairo-xlib-surface.c:471)
   by 0x9F870E6: cairo_surface_finish (cairo-surface.c:724)
   by 0x9F87174: cairo_surface_destroy (cairo-surface.c:645)
   by 0x813C2DD: gdk_window_end_implicit_paint (gdkwindow.c:2836)
   by 0x813C2DD: gdk_window_process_updates_internal (gdkwindow.c:4086)
   by 0x813C537: gdk_window_process_all_updates (gdkwindow.c:4201)
   by 0x7B73805: gtk_container_idle_sizer (gtkcontainer.c:1664)
   by 0x8123A5E: gdk_threads_dispatch (gdk.c:763)
   by 0xB337372: g_main_dispatch (gmain.c:2715)
   by 0xB337372: g_main_context_dispatch (gmain.c:3219)
   by 0xB338E37: g_main_context_iterate (gmain.c:3290)
   by 0xB338E96: g_main_context_iteration (gmain.c:3351)
   by 0xE848D7C: nsAppShell::ProcessNextNativeEvent(bool) (nsAppShell.cpp:212)
   by 0xE82C2F8: nsBaseAppShell::DoProcessNextNativeEvent(bool) (nsBaseAppShell.cpp:138)
   by 0xE82DE9B: nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal*, bool) (nsBaseAppShell.cpp:271)
   by 0xD69B49B: ProcessNextEvent (nsThread.cpp:933)
   by 0xD69B49B: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:849)
   by 0xD6B77F1: NS_ProcessNextEvent(nsIThread*, bool) (nsThreadUtils.cpp:297)
   by 0xD89C589: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:95)
   by 0xD884B71: RunHandler (message_loop.cc:227)
   by 0xD884B71: MessageLoop::Run() (message_loop.cc:201)
   by 0xE82B7F4: nsBaseAppShell::Run() (nsBaseAppShell.cpp:156)
   by 0xEDFB60D: nsAppStartup::Run() (nsAppStartup.cpp:281)
   by 0xEE30068: XREMain::XRE_mainRun() (nsAppRunner.cpp:4290)
   by 0xEE32365: XREMain::XRE_main(int, char**, nsXREAppData const*) (nsAppRunner.cpp:4383)
   by 0xEE325AF: XRE_main (nsAppRunner.cpp:4485)
   by 0x40515C: do_main(int, char**, nsIFile*) (nsBrowserApp.cpp:212)
   by 0x404825: main (nsBrowserApp.cpp:352)

There are some known leaks because the display is not closed due to GTK bugs.

_cairo_freelist_alloc is the kind of memory allocation function where the call stack at first call is not necessarily the leaking call (though it may be).

My guess would be that something has changed the order of calls and so the stack looks different now.  I would look for an existing suppression and modify it and add a similar one to suppress reporting of this leak.
Flags: needinfo?(karlt)
(In reply to Karl Tomlinson (ni?:karlt) from comment #2)
> I would look for an existing suppression and
> modify it and add a similar one to suppress reporting of this leak.

I mean modify *or* add.
(Assignee)

Comment 4

2 years ago
Created attachment 8698914 [details] [diff] [review]
patch

Like this? This seems to successfully suppress the leak on Try, but I'm not 100% sure what I'm doing. In particular, I'm not sure where I'm supposed to truncate the stack.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #8698914 - Flags: review?(karlt)
Comment on attachment 8698914 [details] [diff] [review]
patch

>    fun:gtk_widget_realize
>    fun:_ZN8nsWindow6CreateEP9nsIWidgetPvRKN7mozilla3gfx12IntRectTypedINS4_12UnknownUnitsEEEP16nsWidgetInitData
>    ...

I would just remove these lines from the existing annotation and add a reference to this bug.  I don't know where the format is documented so I don't know whether two "Bug" lines are appropriate, so maybe just note in the comment.

g_main_context_iteration is not very specific anyway, but there are other possible ways of getting to the same code, which may be likely to happen due to other unrelated changes.

The comment can also be updated to indicate suspicion of Gecko not closing the display being the cause.  (The reason for Gecko not doing that is a bug in old GTK3 versions.)
Attachment #8698914 - Flags: review?(karlt) → review+
Blocks: 793882

Comment 6

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/63f78eaa94e8

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/63f78eaa94e8
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.