Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decrement the reference count on the invocation object in _method_callback #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wdouglass
Copy link
Contributor

The caller is responsible for doing this according to:
https://docs.gtk.org/gio/callback.DBusInterfaceMethodCallFunc.html

this should fix #80

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid that dasbus is not the right place for this kind of fix.

@@ -470,6 +470,8 @@ def _method_callback(self, invocation, interface_name, method_name,
method_name,
error
)
finally:
invocation._unref()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a bug in pygobject. Python libraries shouldn't deal with references. I never saw unreferencing in pydbus and it is definitely not happening in dasbus. Could you please report your issue to https://gitlab.gnome.org/GNOME/pygobject?

@wdouglass
Copy link
Contributor Author

Ok, fair enough. I've filed a bug against pygobject here (i've also referenced this patch, i hope that's OK)
https://gitlab.gnome.org/GNOME/pygobject/-/issues/519

@wdouglass
Copy link
Contributor Author

unfortunately, this bug seems to just get kicked back and forth between glib and pygobject.

I'm gonna keep running with the workaround from this PR; i don't think either of those projects are going to bend

@wdouglass
Copy link
Contributor Author

wdouglass commented Dec 20, 2022

As an update, i've written a PR for the pygobject library that also addresses this issue
https://gitlab.gnome.org/GNOME/pygobject/-/merge_requests/219

@poncovka
Copy link
Contributor

poncovka commented Feb 3, 2023

As an update, i've written a PR for the pygobject library that also addresses this issue https://gitlab.gnome.org/GNOME/pygobject/-/merge_requests/219

That is great! Hopefully, it will work out this time. If it gets blocked again please let me know. In that case, I would merge your hotfix for dasbus as a temporary workaround, because I would like to see this issue fixed.

@wdouglass
Copy link
Contributor Author

careful merging this though, because if/when the pygobject fix gets merged, you'll have a ref underflow for systems that have an updated pygobject...

@miccoli
Copy link

miccoli commented Feb 3, 2023

careful merging this though, because if/when the pygobject fix gets merged, you'll have a ref underflow for systems that have an updated pygobject...

Yes: if merged, a version of PyGObject known to have this bug should be pinned, say PyGObject<=3.42.2 (for which I have ample experimental evidence that this patch is working) or PyGObject<=3.43.1 (which I have not yet tested).

By the way, is there a reason for which PyGObject is not listed as a explicit dependency for dasbus?

@poncovka poncovka added the bug Something isn't working label Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Probable memory leak in dasbus server handler
3 participants