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 639597 - attaching a python extension to an object keeps a reference forever
attaching a python extension to an object keeps a reference forever
Status: RESOLVED FIXED
Product: libpeas
Classification: Platform
Component: python
git master
Other Linux
: Normal major
: ---
Assigned To: libpeas-maint
libpeas-maint
Depends on: 644039 644067
Blocks: 626224 626244
 
 
Reported: 2011-01-15 13:45 UTC by Felix Riemann
Modified: 2011-03-10 18:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
instrument peas-demo for the given demo instructions (3.14 KB, patch)
2011-01-15 13:45 UTC, Felix Riemann
reviewed Details | Review
add demo plugin (4.07 KB, patch)
2011-01-18 13:21 UTC, Felix Riemann
none Details | Review
Use dispose() instead of finalize() to unref extension instances. (4.01 KB, patch)
2011-01-18 15:17 UTC, Steve Frécinaux
committed Details | Review
Enable python support for plugins (953 bytes, patch)
2011-03-07 18:06 UTC, Steve Frécinaux
committed Details | Review

Description Felix Riemann 2011-01-15 13:45:32 UTC
Created attachment 178388 [details] [review]
instrument peas-demo for the given demo instructions

We just converted eog over to use libpeas for plugins and it works very well so far for C extensions. It does not however for Python extensions (see bug 626224).

These apparently don't get disposed correctly and keep a reference to the window they were attached to. This however prevents the whole application from shutting down, as that is triggered by the last window's finalize()-handler (which is not run due to the dangling refs). Since eog can be sometimes hard to understand, I'll show you using peas-demo.

1. Apply the attached patch to peas-demo. It will add some debug output regarding the window's ref count and add a dispose handler to clear the extension sets.

2. Run peas-demo, click "New Window".
3. Close the newly generated window.
4. See that dispose and finalize are run. The window object is gone.
5. Click "Quit" and get a warning because the window object is gone.

6. Run peas-demo again, click "New Window"
7. Open the plugin manager and attach the Hello World C plugin.
8. Deactivate it again.
9. Close "Peas Window 1" again, see that the window is disposed and finalized.
10. Click "Quit" and get a warning because the window object is gone.

11. Run peas-demo again, click "New Window"
12. Open the plugin manager and attach the Python plugin.
13. Deactivate the plugin.
14. Close "Peas Window 1", see that the window is disposed but not finalized.
15. Click "Quit" and see that there are still two references held on the window.
Comment 1 Steve Frécinaux 2011-01-17 15:02:15 UTC
Do you have a python plugin example for eog?
Comment 2 Felix Riemann 2011-01-18 13:21:28 UTC
Created attachment 178623 [details] [review]
add demo plugin

This adds a demo plugin based on the one from bug 639597 (I think it's based on
libpeas' demo plugin) that I used for testing.

It spits warnings now when the window is closed, maybe something changed in
gtk+ again. If these concern you can comment out the eog functionality and just
keep the printfs. It should still fail to release the window ref then.
Comment 3 Steve Frécinaux 2011-01-18 15:17:48 UTC
Created attachment 178630 [details] [review]
Use dispose() instead of finalize() to unref extension instances.

This fixes refcount issues with non-native extensions, which prevented
objects which had extensions attached to ever get finalized.
Comment 4 Steve Frécinaux 2011-01-18 15:18:38 UTC
Felix, can you tell me if applying the above patch fixes the problem for you?
Comment 5 Felix Riemann 2011-01-18 15:49:16 UTC
(In reply to comment #4)
> Felix, can you tell me if applying the above patch fixes the problem for you?

No, unfortunately not (at least for python). eog application still stays alive.
peas-demo also  says there are still two refs left.
Comment 6 Felix Riemann 2011-01-18 15:52:58 UTC
Hmm, the ref might not necessarily leak in the python extension itself. Activating and deactivating the python plugin twice doesn't increase the refcount to 4 but keeps it at 2.
Comment 7 Steve Frécinaux 2011-01-18 16:20:33 UTC
Actually, the patch here fixes the issue in peas-demo (the window gets finalized, at least here, using libpeas master), but neither is it in eog nor in gedit. Gedit quits anyway because they are using some other mecanism to detect the end of the program.
Comment 8 Steve Frécinaux 2011-01-18 16:59:53 UTC
Ok, so it looks like even in peas-demo there is some leaked ref or something.

Basically we have run_dispose() which runs dispose() twice in the regular plugin case, and for some reason it runs only once in the python plugin case. Also some references are still there and finalize() is never called.

Window closing with a c plugin enabled:

DemoWindow dispose >> 5
** (peas-demo:22198): DEBUG: peasdemo_hello_world_plugin_deactivate
** (peas-demo:22198): DEBUG: peasdemo_hello_world_plugin_finalize
DemoWindow disposed >> 2
DemoWindow dispose >> 1
DemoWindow disposed >> 1
DemoWindow finalize >> 0

Window closing with a python plugin enabled:

DemoWindow dispose >> 6
PythonHelloPlugin.do_deactivate <__main__.DemoWindow object at 0x907bc0c (DemoWindow at 0x8f52098)>
PeasExtensionPython dispose
PeasExtensionPython finalize
DemoWindow disposed >> 4
Comment 9 Steve Frécinaux 2011-01-18 18:03:14 UTC
Ok I think I found the culprit.

The issue is that we have one extra reference for the python extension itself. This reference is added for some reason by pygobject_constructv(), and I need to figure out why. Apparently it shouldn't ;-)

Since there is one python ref too much, it's never released, so I guess the extension keeps one ref to the window...
Comment 10 Steve Frécinaux 2011-01-20 10:05:18 UTC
Bug 639949 fixes half the issue at the pygobject level: pygobject did leak references on GtkWindow objects.

This doesn't fix the reference counting issue yet but there is one less leaked reference. I still have to look at the potentially leaked python reference so I hope this will be fixed today.
Comment 11 Steve Frécinaux 2011-01-20 16:14:37 UTC
Comment on attachment 178630 [details] [review]
Use dispose() instead of finalize() to unref extension instances.

I'm pushing the dispose() patch already as it's the right thing to do anyway.

Attachment 178630 [details] pushed as f3f6b4e - Use dispose() instead of finalize() to unref extension instances.
Comment 12 Steve Frécinaux 2011-03-06 20:04:26 UTC
Ok, so as of today the situation is as follow:

There are two leaked references in pygobject.
- the first one is leaked because pygobject will add one extra reference when
  creating a wrapper around a GInitiallyUnowned instance, thinking he owns that
  instance and, as such, needs to ref_sink it. I don't think this is fixable
  without breaking the static bindings, but it might be worked around to some
  extent in libpeas.

- the second one is due to the property helper gobject.property(), which for
  some reason indefinitely holds a reference to the object. It can be worked
  around by providing custom setter and getter to the property to keep the
  value within the class, which solves the leaked ref. It should be fixed in
  pygobject though.
Comment 13 Steve Frécinaux 2011-03-07 01:14:45 UTC
Everything works as expected when applying patches attached to bug 644039 and bug 644067
Comment 14 Steve Frécinaux 2011-03-07 18:06:45 UTC
Created attachment 182743 [details] [review]
Enable python support for plugins
Comment 15 Felix Riemann 2011-03-08 14:36:24 UTC
Review of attachment 182743 [details] [review]:

Thanks for handling this, Steve.
eog seems to behave fine now. :)

commit d1d6b9452acc874d1f46ca57f5695bf128acbf91
Author: Steve Frécinaux <>
Date:   Mon Mar 7 19:05:44 2011 +0100

    Enable python support for plugins
    
    https://bugzilla.gnome.org/show_bug.cgi?id=639597
Comment 16 Steve Frécinaux 2011-03-10 18:14:14 UTC
Ok I'm closing this bug as pygobject 2.8 contains the above-mentioned patches. In turn libpeas 0.7.5 depends on pygobject 2.8.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.