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 794207 - leak: g_socket_listener_add_inet_port increases ref-count on socket-listener
leak: g_socket_listener_add_inet_port increases ref-count on socket-listener
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.54.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2018-03-09 15:37 UTC by Dirk-Jan C. Binnema
Modified: 2018-03-14 17:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gsocketlistener: Fix a typo in the documentation (968 bytes, patch)
2018-03-13 14:06 UTC, Philip Withnall
committed Details | Review
gsocketlistener: Document the need to call g_socket_listener_close() (2.46 KB, patch)
2018-03-13 14:06 UTC, Philip Withnall
committed Details | Review

Description Dirk-Jan C. Binnema 2018-03-09 15:37:15 UTC
It seems that g_socket_listener_add_inet_port unexpectedly increases the ref-count of the listener:

--------------------------
#include <gio/gio.h>

int
main(int argc, char *argv[])
{
	GSocketListener *listener;
	
        listener = G_SOCKET_LISTENER(g_socket_service_new());
        if (!g_socket_listener_add_inet_port (listener, 9999, NULL, NULL))
		g_warning ("failed to add port");

	g_object_unref (listener); /* expected:   succeeds, no error */
	g_object_unref (listener); /* unexpected:_also_ succeeds, no error ! */
	
	return 0;
}
-------------------------

it seems this happens only once, ie. multiple calls g_socket_listener_add_inet_port do not add additional refs.
Comment 1 Dirk-Jan C. Binnema 2018-03-13 06:37:37 UTC
A bit of gdb suggests the extra ref is coming from g_socket_listener_accept_async, which starts a GTask that adds a ref.

That task is ongoing when unreffing the listener, so it won't be finalized; not sure if the gtask will eventually (timeout?) cancel, but it looks like a circular dependency.

One visible side-effect of this is that a thought-to-be-dead service may still listen on some port, so subsequent instances that want to bind that same port will fail (this how this problem showed up in the first place), signal handlers will still be called etc.
Comment 2 Dan Winship 2018-03-13 13:51:43 UTC
> One visible side-effect of this is that a thought-to-be-dead service may
> still listen on some port, so subsequent instances that want to bind that
> same port will fail

You can/should call g_socket_listener_close() if you want to stop the listener. This could probably be documented better. (Patch welcome.)
Comment 3 Philip Withnall 2018-03-13 14:06:05 UTC
Created attachment 369621 [details] [review]
gsocketlistener: Fix a typo in the documentation

Mismatched singular/plural.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 4 Philip Withnall 2018-03-13 14:06:15 UTC
Created attachment 369622 [details] [review]
gsocketlistener: Document the need to call g_socket_listener_close()

GSocketListener can keep internal references to itself for pending
accept() calls, which mean that it can stay alive (and keep listening
on ports) even after a user drops their last reference to it. They need
to call g_socket_listener_close() explicitly to avoid that.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 5 Dirk-Jan C. Binnema 2018-03-13 20:13:04 UTC
Thanks for the quick update! The documentation could perhaps also mention that one needs to manually disconnect any signal handlers (I ran into that issue).
Comment 6 Philip Withnall 2018-03-13 21:03:25 UTC
(In reply to Dirk-Jan C. Binnema from comment #5)
> The documentation could perhaps also mention
> that one needs to manually disconnect any signal handlers (I ran into that
> issue).

That’s just GLib best practice. Always disconnect signal handlers from an object when you drop your last reference to it.
Comment 7 Philip Withnall 2018-03-14 17:23:13 UTC
Pushed to master, thanks.

Attachment 369621 [details] pushed as dd815c3 - gsocketlistener: Fix a typo in the documentation
Attachment 369622 [details] pushed as d107730 - gsocketlistener: Document the need to call g_socket_listener_close()