After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756431 - goa-identity-service memory leak
goa-identity-service memory leak
Status: RESOLVED FIXED
Product: gnome-online-accounts
Classification: Core
Component: Kerberos
3.18.x
Other All
: Normal normal
: ---
Assigned To: GNOME Online Accounts maintainer(s)
GNOME Online Accounts maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-10-12 12:32 UTC by Bastien Nocera
Modified: 2016-09-14 19:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
identity: Don't leak the invocation when handling SignIn (846 bytes, patch)
2016-07-18 13:36 UTC, Debarshi Ray
committed Details | Review
identity: Don't leak the GoaObject from find_object_with_principal (3.13 KB, patch)
2016-07-30 17:08 UTC, Debarshi Ray
committed Details | Review
[gnome-3-20] identity: Don't leak the identifier in handle_sign_in (1.13 KB, patch)
2016-08-02 00:09 UTC, Debarshi Ray
committed Details | Review

Description Bastien Nocera 2015-10-12 12:32:01 UTC
[1037566.002051] Out of memory: Kill process 2418 (goa-identity-se) score 96 or sacrifice child
 [1037566.002054] Killed process 2418 (goa-identity-se) total-vm:1321880kB, anon-rss:757896kB, file-rss:0kB

That's 750 megs of RSS for the goa-identity-service helper. I was trying to log into the Red Hat Intranet, which uses Kerberos for one-time login, through Firefox.

gnome-online-accounts-3.18.0-1.fc23.x86_64
Comment 1 Bastien Nocera 2015-10-26 00:03:12 UTC
Still seeing this in gnome-online-accounts-3.18.1-1.fc23.x86_64
Comment 2 Debarshi Ray 2016-07-14 14:41:59 UTC
I left goa-identity-service running under valgrind overnight only to find that the machine has rebooted. Happened twice on two attempts. I don't see anything in the journal for the last or current boot, no crashes.

I wonder what's going on?

Do you remember whether you had gotten a ticket through Settings -> Online Accounts? Or was it through kinit on the command line?
Comment 3 Bastien Nocera 2016-07-18 11:22:29 UTC
(In reply to Debarshi Ray from comment #2)
> I left goa-identity-service running under valgrind overnight only to find
> that the machine has rebooted. Happened twice on two attempts. I don't see
> anything in the journal for the last or current boot, no crashes.
> 
> I wonder what's going on?
> 
> Do you remember whether you had gotten a ticket through Settings -> Online
> Accounts? Or was it through kinit on the command line?

I honestly don't remember for that particular one. Nowadays, I have more RAM, and goa-identity-service is probably still leaky:
hadess    6128  0.3  3.7 1045812 603696 ?      SLl  Jul12  34:25 /usr/libexec/goa-identity-service

I don't use kinit but logged in through the Online Accounts panel a long while ago, I've connected and disconnected from the VPN a number of times.
Comment 4 Debarshi Ray 2016-07-18 11:58:34 UTC
(In reply to Bastien Nocera from comment #3)
> I honestly don't remember for that particular one. Nowadays, I have more
> RAM, and goa-identity-service is probably still leaky:
> hadess    6128  0.3  3.7 1045812 603696 ?      SLl  Jul12  34:25
> /usr/libexec/goa-identity-service

Ok.

> I don't use kinit but logged in through the Online Accounts panel a long
> while ago, I've connected and disconnected from the VPN a number of times.

Does your /proc/keys keep growing? I just want to make sure that this is not bug 768808
Comment 5 Bastien Nocera 2016-07-18 12:26:27 UTC
$ cat /proc/keys | wc -l
15

Seems reasonable.
Comment 6 Debarshi Ray 2016-07-18 13:36:58 UTC
Created attachment 331710 [details] [review]
identity: Don't leak the invocation when handling SignIn

Found it while testing Christophe's GTask patches in bug 764157 This is probably not the leak you are suffering from because this code path is only run when initially creating the account, but still.

I wonder if this is hit when the ticket expires. In that case it will be relevant. Need to check.
Comment 7 Ray Strode [halfline] 2016-07-18 15:42:07 UTC
Review of attachment 331710 [details] [review]:

seems fine to me.  this is like bug 767952 exposing itself in the wild.

Alternatively, we could just drop the extra g_object_ref we're doing when first getting the invocation in?
Comment 8 Debarshi Ray 2016-07-18 16:05:18 UTC
(In reply to Ray Strode [halfline] from comment #7)
> Alternatively, we could just drop the extra g_object_ref we're doing when
> first getting the invocation in?

True. A small part of me likes to retain the ref/unref for the sake of consistency with the usual GObject practices.

Thanks for the review, by the way!
Comment 9 Debarshi Ray 2016-07-18 16:12:00 UTC
Comment on attachment 331710 [details] [review]
identity: Don't leak the invocation when handling SignIn

Pushed to master, gnome-3-20 and gnome-3-18. Let's keep this open for a bit to see if we can catch a few more.
Comment 10 Debarshi Ray 2016-07-30 17:08:28 UTC
Created attachment 332403 [details] [review]
identity: Don't leak the GoaObject from find_object_with_principal
Comment 11 Ray Strode [halfline] 2016-08-01 14:51:10 UTC
Review of attachment 332403 [details] [review]:

eek, that's quite a leak! It seems only one caller cleaned up the result.
Comment 12 Debarshi Ray 2016-08-01 21:39:25 UTC
Comment on attachment 332403 [details] [review]
identity: Don't leak the GoaObject from find_object_with_principal

Pushed to master, gnome-3-20 and gnome-3-18 (which also needed commit 09e110375f3c1).
Comment 13 Debarshi Ray 2016-08-02 00:09:34 UTC
Created attachment 332497 [details] [review]
[gnome-3-20] identity: Don't leak the identifier in handle_sign_in

One side-effect of commit b7cc0ded1f13fa was to stop leaking the identifier. We don't seem to be using it via the source_tag anywhere. This splits out the leak fix for older branches.
Comment 14 Debarshi Ray 2016-09-14 19:34:34 UTC
Comment on attachment 332497 [details] [review]
[gnome-3-20] identity: Don't leak the identifier in handle_sign_in

Pushed to gnome-3-20.
Comment 15 Debarshi Ray 2016-09-14 19:37:42 UTC
I am closing this now. Please re-open or file a new bug if the leaks persist.