Closed Bug 1228724 Opened 4 years ago Closed 4 years ago

Valgrind leak when the window is maximized

Categories

(Core :: Widget: Gtk, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox45 --- affected
firefox46 --- fixed

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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?
Flags: needinfo?(karlt)
(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.
Attached patch patchSplinter Review
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+
https://hg.mozilla.org/mozilla-central/rev/63f78eaa94e8
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.