Bug 223937 - [patch] sysutils/xfce4-wavelan-plugin fix memory leak due to CSS changes
Summary: [patch] sysutils/xfce4-wavelan-plugin fix memory leak due to CSS changes
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Guido Falsi
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-11-28 13:12 UTC by J.R. Oldroyd
Modified: 2017-12-14 15:03 UTC (History)
2 users (show)

See Also:
madpilot: maintainer-feedback+
madpilot: merge-quarterly+


Attachments
patch to fix memory leak due to Gtk3 CSS changes (3.60 KB, patch)
2017-11-28 13:12 UTC, J.R. Oldroyd
no flags Details | Diff
patch to fix mem leak in similar way to forthcoming upstream changes (2.84 KB, patch)
2017-11-29 02:42 UTC, J.R. Oldroyd
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description J.R. Oldroyd 2017-11-28 13:12:21 UTC
Created attachment 188351 [details]
patch to fix memory leak due to Gtk3 CSS changes

The recent upstream changes to support Gtk3 CSS introduce a rampant memory leak because the css_provider is re-created every time through the loop.  Attached patch ensures that the css_provider is only created once.

I am attaching a FreeBSD port patch as this memory leak is significant and needs an immediate fix.

I will also submit a bug report upstream in order to have this problem fixed there, too.
Comment 1 Guido Falsi freebsd_committer 2017-11-28 15:22:22 UTC
Hi,

Thanks for the patch.

Just to make sure I understand correctly, the "recent upstream changes" are in the GTK3 sources?

Since you are sending this upstream too, could you followup with a link to the upstream bug so it can be tracked? It would allow avoiding to commit patches different from what is accepted upstream.

In the while I'll also test it.
Comment 2 Guido Falsi freebsd_committer 2017-11-28 16:44:53 UTC
I noticed there is already an upstream bug report about this:

https://bugzilla.xfce.org/show_bug.cgi?id=13495

No patch attached though, maybe you should followup here.
Comment 3 Olivier Duchateau 2017-11-28 17:01:54 UTC
(In reply to Guido Falsi from comment #1)

It's just upstream patch [1], from upcoming release.

[1] https://git.xfce.org/panel-plugins/xfce4-wavelan-plugin/commit/?id=d4c76389d63199636c52e8a7a0ce80d5f2fde591
Comment 4 Olivier Duchateau 2017-11-28 17:02:46 UTC
(In reply to Guido Falsi from comment #2)

All our patches are already merged by upstream.
Comment 5 Guido Falsi freebsd_committer 2017-11-28 17:45:18 UTC
(In reply to Olivier Duchateau from comment #4)
> (In reply to Guido Falsi from comment #2)
> 
> All our patches are already merged by upstream.

Thank you, I did not see that.

In general always give pointers to upstream bugs or (even better) commits in ports bug reports, this speeds up things.

Looking at the commit the upstream code did get a refactoring, and is doing things differently.

Wouldn't be importing the upstream commit unchanged better?

That commit applies with minimal changes and should be equivalent to your patch.

(my poudriere machine is busy right now, but I'm going to test what I propose shortly, if it works out fine I'll send a revised patch)
Comment 6 J.R. Oldroyd 2017-11-28 17:53:11 UTC
Yes, I had not seen the upstream code refactoring when I did my patch earlier.  (Sorry, should'a looked, I know!)

I am now also testing the upstream version here.  Agreed that we should use that if it does work, along with our FreeBSD-specific patch.

My initial testing with the upstream version has seen the process' RSS increase in 4k steps by a total of 16k over the first half hour or so, after which it appears to have stopped growing.  So we are probably good with that.
Comment 7 J.R. Oldroyd 2017-11-29 02:42:45 UTC
Created attachment 188385 [details]
patch to fix mem leak in similar way to forthcoming upstream changes

This is a revised files/patch-panel-plugin__wavelan.c that fixes the memory leak in a manner consistent with forthcoming upstream changes.  It retains our prior FreeBSD-specific changes that were already in the patch, but does not bring in further upstream changes that we don't already have.
Comment 8 commit-hook freebsd_committer 2017-11-30 20:37:43 UTC
A commit references this bug:

Author: madpilot
Date: Thu Nov 30 20:36:35 UTC 2017
New revision: 455225
URL: https://svnweb.freebsd.org/changeset/ports/455225

Log:
  - Import upstream patch to fix a memory leak [1]
  - While here rename and regenerate patches

  PR:		223937 [1]
  Submitted by:	J.R. Oldroyd <fbsd@opal.com>
  Obtained from:	https://git.xfce.org/panel-plugins/xfce4-wavelan-plugin/commit/?id=d4c76389d63199636c52e8a7a0ce80d5f2fde591
  MFH:		2017Q4

Changes:
  head/sysutils/xfce4-wavelan-plugin/Makefile
  head/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin__wavelan.c
  head/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin__wi_bsd.c
  head/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin_wavelan.c
  head/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin_wi__bsd.c
Comment 9 commit-hook freebsd_committer 2017-12-14 15:02:10 UTC
A commit references this bug:

Author: madpilot
Date: Thu Dec 14 15:01:22 UTC 2017
New revision: 456302
URL: https://svnweb.freebsd.org/changeset/ports/456302

Log:
  MFH: r455225

  - Import upstream patch to fix a memory leak [1]
  - While here rename and regenerate patches

  PR:		223937 [1]
  Submitted by:	J.R. Oldroyd <fbsd@opal.com>
  Obtained from:	https://git.xfce.org/panel-plugins/xfce4-wavelan-plugin/commit/?id=d4c76389d63199636c52e8a7a0ce80d5f2fde591

  Approved by:	ports-secteam (swills)

Changes:
_U  branches/2017Q4/
  branches/2017Q4/sysutils/xfce4-wavelan-plugin/Makefile
  branches/2017Q4/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin__wavelan.c
  branches/2017Q4/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin__wi_bsd.c
  branches/2017Q4/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin_wavelan.c
  branches/2017Q4/sysutils/xfce4-wavelan-plugin/files/patch-panel-plugin_wi__bsd.c
Comment 10 Guido Falsi freebsd_committer 2017-12-14 15:03:34 UTC
Commited and merged! Thanks.