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 748646 - spice: do not leak channels on open-fd
spice: do not leak channels on open-fd
Status: RESOLVED DUPLICATE of bug 746800
Product: gnome-boxes
Classification: Applications
Component: spice
unspecified
Other All
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-04-29 14:04 UTC by Marc-Andre Lureau
Modified: 2016-09-20 08:15 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
spice: do not leak channels on open-fd (1.59 KB, patch)
2015-04-29 14:05 UTC, Marc-Andre Lureau 		reviewed Details | Review

Description Marc-Andre Lureau 2015-04-29 14:04:55 UTC 
Vala closures will keep a reference on the objects they use. Make sure
to disconnect the signal handler to dispose the closure.

This fix leaks and may solve some issues when channels are kept around
(such as Spice audio session still running because channels are alive)
Comment 1 Marc-Andre Lureau 2015-04-29 14:05:04 UTC 
Created attachment 302569 [details] [review]
spice: do not leak channels on open-fd
Comment 2 Zeeshan Ali 2015-04-29 14:57:10 UTC 
Review of attachment 302569 [details] [review]:

::: src/spice-display.vala
@@ +212,3 @@
                         fd = open_fd ();
                         channel.open_fd (fd);
+                        SignalHandler.disconnect (channel, open_fd_id);

probably a stupid question but we are guaranteed that spice will only want one fd per channel?
Comment 3 Zeeshan Ali 2015-04-29 14:57:36 UTC 
Review of attachment 302569 [details] [review]:

::: src/spice-display.vala
@@ +212,3 @@
                         fd = open_fd ();
                         channel.open_fd (fd);
+                        SignalHandler.disconnect (channel, open_fd_id);

we are -> are we :)
Comment 4 Marc-Andre Lureau 2015-04-29 15:13:20 UTC 
you are right, it's not guaranteed. However, that was simplest fix I could do to fix the leak. Perhaps there is a way to tell vala not to take a strong reference on the channel?
Comment 5 Zeeshan Ali 2015-04-29 15:17:37 UTC 
(In reply to Marc-Andre Lureau from comment #4)
> you are right, it's not guaranteed. However, that was simplest fix I could
> do to fix the leak. Perhaps there is a way to tell vala not to take a strong
> reference on the channel?

Perhaps by not using the channel from the closure but the channel param passed to the signal handler?
Comment 6 Zeeshan Ali 2015-05-07 15:04:03 UTC 
Lets handle all these spice related leaks in bug#746800.

*** This bug has been marked as a duplicate of bug 746800 ***