Hi, On Tue, Sep 04, 2018 at 02:11:46PM +0200, Jakub Janku wrote: > > > As a result, the reference to GIOStream is not released and > > > the FD is not closed. > > > > > > So the problem aren't the references to VDAgentConnection > > > itself, but rather the GLib's internal references to the > > > GIOStream we use in VDAgentConnection. > > > > If we follow glib's API we either should be fine or find a > > bug in glib > > It's not that simple I'm afraid. > Have a look at https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html > and the diagram ( > https://developer.gnome.org/glib/stable/mainloop-states.gif ) > When signal_handler() is called, the main loop is in the "dispatching" > phase. We can call vdagent_virtio_port_destroy() in the signal > handler, but the problem is that if we call g_main_loop_quit(), the > main loop won't iterate again. > > And we need that extra iterations because GCancellable uses > file descriptor and hence the main loop needs to go through > that "polling" phase so that we get the > GError::G_IO_ERROR_CANCELLED error. > > So in other words, if the loop is quit too early, the > GIOStream's callback won't be called and connection won't be > finalized. How are you checking that any FD is left open? The session agent would not run without the system one, so it shouldn't be a problem in case of terminating the process. The documentation for this GSource points out that a common use case is to flush things and call g_main_loop_quit(). https://developer.gnome.org/glib/stable/glib-UNIX-specific-utilities-and-integration.html#g-unix-signal-source-new Anyway, if needed, you can add a g_idle_add() to g_main_loop_quit with some comment /* to be sure that GIOStream is closed */ considering that you have called the g_cancellable_cancel(), it should be plenty. > Cheers, > Jakub > > > > > > I hope this makes it a bit more clear. > > > > Let me know if I get it wrong again ;) > > > > Cheers, > > Victor > > > > > Thanks for the reviews :) > > > Jakub > > > > > > > > > > > > > Jakub Janků (18): > > > > > vdagentd: parse argv using GLib > > > > > vport: add by_user param to vdagent_virtio_port_disconnect_callback > > > > > vdagentd: use GMainLoop > > > > > build: add GIO dependency > > > > > add VDAgentConnection > > > > > udscs: add udscs_get_peer_pid() > > > > > udscs: use VDAgentConnection > > > > > udscs-server: split initialization > > > > > udscs: simplify logging > > > > > vport: use VDAgentConnection > > > > > session-info: add ActiveSessionChangeCb > > > > > console-kit: use GDBus > > > > > systemd-login: use GDBus > > > > > session-info: remove session_info_get_fd() > > > > > build: drop DBus dependency > > > > > move to GLib memory functions > > > > > vdagentd: move code to do_guest_xorg_resolution() > > > > > vdagentd: move code to do_agent_file_xfer_status() > > > > > > > > > > Makefile.am | 6 +- > > > > > configure.ac | 2 +- > > > > > src/udscs.c | 560 +++++++-------------------- > > > > > src/udscs.h | 58 +-- > > > > > src/vdagent-connection.c | 301 +++++++++++++++ > > > > > src/vdagent-connection.h | 103 +++++ > > > > > src/vdagent/vdagent.c | 3 +- > > > > > src/vdagent/x11-randr.c | 43 +-- > > > > > src/vdagentd/console-kit.c | 613 ++++++++++-------------------- > > > > > src/vdagentd/dummy-session-info.c | 7 +- > > > > > src/vdagentd/session-info.h | 6 +- > > > > > src/vdagentd/systemd-login.c | 274 +++++-------- > > > > > src/vdagentd/uinput.c | 9 +- > > > > > src/vdagentd/vdagentd.c | 472 +++++++++++------------ > > > > > src/vdagentd/virtio-port.c | 404 ++++++-------------- > > > > > src/vdagentd/virtio-port.h | 27 +- > > > > > 16 files changed, 1217 insertions(+), 1671 deletions(-) > > > > > create mode 100644 src/vdagent-connection.c > > > > > create mode 100644 src/vdagent-connection.h > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > > > _______________________________________________ > > > > > Spice-devel mailing list > > > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel