Bug 1180886

Summary: polkit doesn't release reference counters of GVariant data for org.freedesktop.PolicyKit1.Authority.EnumerateActions dbus call.
Product: [Fedora] Fedora Reporter: Rex Dieter <rdieter>
Component: polkitAssignee: Miloslav Trmač <mitr>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 20CC: christopherheiny, felipe, martin.marques, mitr
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: polkit-0.112-7.fc20.1 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-02-08 08:58:20 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 871761    

Description Rex Dieter 2015-01-11 14:09:45 UTC 
Been tracking an apparent memory leak in kde for awhile, and it would appear the root cause may be stemming from org.freedesktop.PolicyKit1.Authority.EnumerateActions dbus calls.

See bug #871761 and upstream https://bugs.kde.org/show_bug.cgi?id=271934 for some history.

One user in https://bugs.kde.org/show_bug.cgi?id=271934#c121 comments:

It seems, that the problem is not in powerdevil, nor in KDE (and it's libraries) at all.
I've found, that policykit-1-0.105 in my Debian Wheezy doesn't release reference counters of GVariant data for org.freedesktop.PolicyKit1.Authority.EnumerateActions dbus call. 

So in my case following patch solves my kded4 with enabled powerdevil extreme memory leak:
--- policykit-1-0.105.orig/src/polkit/polkitauthority.c
+++ policykit-1-0.105/src/polkit/polkitauthority.c
@@ -715,7 +715,6 @@
   while ((child = g_variant_iter_next_value (&iter)) != NULL)
     {
       ret = g_list_prepend (ret, polkit_action_description_new_for_gvariant (child));
-      g_variant_ref_sink (child);
       g_variant_unref (child);
     }
   ret = g_list_reverse (ret);

After removing g_variant_ref_sink(child), rebuilding policykit-1-0.105 packages, their reinstallation and kded4 restart with 'kquitapp kded; kded4' I can't see any memory leak. jemalloc profiling and valgrind confirmed this.
Comment 1 Miloslav Trmač 2015-01-12 23:05:08 UTC 
Thanks for the patch; I didn’t test this yet but looking at API documentation, that must be it.

(Should also audit all the floating reference usage of GVariants; several of the other g_variant_ref_sink()/g_variant_ref_unref() calls seem unnecessary.)
Comment 2 Rex Dieter 2015-01-13 13:46:20 UTC 
Looks like it was upstreamed quickly,

http://cgit.freedesktop.org/polkit/commit/?id=f4d71e0de885010494b8b0b8d62ca910011d7544


I can help prepare patched polkit builds for bodhi, if you like.
Comment 3 Rex Dieter 2015-01-25 19:33:57 UTC 
upstream is asking for packages to test, so I'll go ahead and do that with the minimal fix for now.
Comment 4 Rex Dieter 2015-01-25 20:03:31 UTC 
Imported polkit upstream commit into

%changelog
* Sun Jan 25 2015 Rex Dieter <rdieter>  - 0.112-9
- polkit doesn't release reference counters of GVariant data (#1180886)
- fix ldconfig scriptlets (move to -libs subpkg)

for rawhide.


I was hesitant to merge in back to f21/f20 branches largely due to prior commit:

* Sat Nov 08 2014 Colin Walters <walters> - 0.112-8
- Split separate -libs package, so that NetworkManager can just depend on
  that, without dragging in the daemon (as well as libmozjs17).  This
  allows the creation of more minimal systems that want programs like NM,
  but do not need the configurability of the daemon; it would be ok if only
  root is authorized.

which you may or may not want included in f21/f20 builds.  Do you care either way?
Comment 5 Miloslav Trmač 2015-01-26 22:00:07 UTC 
(In reply to Rex Dieter from comment #4)
> Imported polkit upstream commit into
> 
> %changelog
> * Sun Jan 25 2015 Rex Dieter <rdieter>  - 0.112-9
> - polkit doesn't release reference counters of GVariant data (#1180886)
> - fix ldconfig scriptlets (move to -libs subpkg)
> 
> for rawhide.
Thanks.

> I was hesitant to merge in back to f21/f20 branches largely due to prior
> commit:
> 
> * Sat Nov 08 2014 Colin Walters <walters> - 0.112-8
> - Split separate -libs package, so that NetworkManager can just depend on
>   that, without dragging in the daemon (as well as libmozjs17).  This
>   allows the creation of more minimal systems that want programs like NM,
>   but do not need the configurability of the daemon; it would be ok if only
>   root is authorized.
> 
> which you may or may not want included in f21/f20 builds.  Do you care
> either way?

This has a risk of breaking kickstarts (by no longer pulling in polkitd and the utilities through a libpolkit*.so dependency) so I’d prefer to leave this split for F>=22 only.
Comment 6 Rex Dieter 2015-01-27 14:55:30 UTC 
OK, how about merging f21 branch back to f20 ?

In particular, do you see any problem including these commits for f20 build? (these look safe to me)

commit 56a2b8b4c5eac65bfaf57877b4753f2adb6cb60f
Author: Kay Sievers <kay>
Date:   Thu Jun 5 17:17:06 2014 +0200

    Backport upstream D-Bus "user bus" changes

commit f3502e13349d97b0a5648cf92cf68e98dfc091c3
Author: Miloslav Trmač <mitr>
Date:   Tue Feb 11 11:09:04 2014 +0100

    Fix a PolkitAgentSession race condition


Or would you rather I backport the single patch (referenced in comment #2) for each branch separately?
Comment 7 Fedora Update System 2015-01-27 15:37:07 UTC 
polkit-0.112-7.fc21.1 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/polkit-0.112-7.fc21.1
Comment 8 Fedora Update System 2015-01-27 15:51:09 UTC 
polkit-0.112-7.fc20.1 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/polkit-0.112-7.fc20.1
Comment 9 Fedora Update System 2015-01-28 20:00:18 UTC 
Package polkit-0.112-7.fc20.1:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing polkit-0.112-7.fc20.1'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2015-1285/polkit-0.112-7.fc20.1
then log in and leave karma (feedback).
Comment 10 Martí­n Marqués 2015-02-02 14:49:40 UTC 
Not sure if my problem is relates (I thought it was), but last update didn't fix my kded not responding to sleep and hibernate commands.

I get this from kded:

Error response:  "Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken." 

Solution for when this happens is to kill kded (with a SIGKILL, others don't work) and start it again.
Comment 11 Rex Dieter 2015-02-02 14:50:56 UTC 
Your problem is likely unrelated, this one is largely about memory leaks only
Comment 12 Fedora Update System 2015-02-04 08:03:19 UTC 
polkit-0.112-7.fc21.1 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 13 Fedora Update System 2015-02-08 08:58:20 UTC 
polkit-0.112-7.fc20.1 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 14 Christopher Heiny 2015-02-28 20:56:08 UTC 
I'm still experiencing this on Fedora 20, with polkit-0.112-7.fc20.1.x86_64 and kdelibs-4.14.4-2.fc20.x86_64.  Or at least, something with identical symptoms (bloated, unresponsive kded4).  Doing

    killall -HUP kded4

results in about a minute or so of furious CPU activity (100% of one core), and then either:
    a) things work OK for a while (unlikely); or
    b) kded4 eventually goes away (most common).

Anyway, should I open a new bug for this?  What information can I provide to help debug it?

Thanks!
Comment 15 Rex Dieter 2015-03-01 15:40:21 UTC 
Yes, new bug please (I see you've already poked the upstream bug too, good).  The leak could theoretically be in many places, but we're fairly certain *this* one is handled now.