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

Remove unused calls to xdpw_request_create #283

Closed
wants to merge 1 commit into from

Conversation

r3dey3
Copy link

@r3dey3 r3dey3 commented Sep 7, 2023

The req object was not used and these calls were resulting with an error when attempting the pick-color from kdenlive on ubuntu 22.04 and 23.04

These objects were also not freed which is a memory leak.

The req object was not used and these calls were resulting with an error
when attempting the pick-color from kdenlive on ubuntu 22.04 and 23.04

These objects were also not freed which is a memory leak.
@emersion
Copy link
Owner

emersion commented Sep 8, 2023

Hm, I was under the impression that allocating D-Bus objects for these handles was important...

@r3dey3
Copy link
Author

r3dey3 commented Sep 9, 2023

So I went and read through a bit of the docs (https://docs.flatpak.org/en/latest/portal-api-reference.html#gdbus-org.freedesktop.portal.Request) and other code - it looks like the code is supposed to respond with the Request object, then do the action (and if the Close method is called, cancel); then finally free up the request object.

I think the main issue why things aren't working is multi faceted -

  1. The request object isn't actually sent in a reply - "the reply includes a handle (i.e. object path) for a Request object"
  2. "handle... which will stay alive for the duration of the user interaction" - the request object needs to be released at the end of the user interaction
  3. The code doesn't process dbus messages while waiting for user input - so the close method wouldn't be processed anyways.

Feel free to reject this pull request :-)

@emersion
Copy link
Owner

Superseded by #307

@emersion emersion closed this May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants