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 698640 - Does not call dbus_connection_close on cleanup
Does not call dbus_connection_close on cleanup
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal minor
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-04-23 09:21 UTC by Martin Pitt
Modified: 2013-05-03 15:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
WIP: Close private D-BUS connections (1.42 KB, patch)
2013-04-23 09:41 UTC, Martin Pitt
reviewed Details | Review

Description Martin Pitt 2013-04-23 09:21:24 UTC
While running integration tests against current git head, I noticed this message on shutdown of NM:

------------- 8< --------------
NetworkManager[31415]: <info> caught signal 15, shutting down normally.
process 31415: The last reference on a connection was dropped without closing the connection. This is a bug in an application. See dbus_connection_unref() documentation for details.
Most likely, the application was supposed to call dbus_connection_close(), since this is a private connection.
this watch should have been invalidatedthis watch should have been invalidatedNetworkManager[31415]: <info> exiting (success)
------------- 8< --------------

I think this is a regression from http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=a87b5a15df45d7974c942ffa6bc4a5453c269d4e . In the case of a private connection, dbus_connection_unref() isn't enough, one needs to dbus_connection_close() the connection first. See http://dbus.freedesktop.org/doc/api/html/group__DBusConnection.html#ga6385ff09bc108238c4429e7c195dab25
Comment 1 Martin Pitt 2013-04-23 09:41:18 UTC
Created attachment 242197 [details] [review]
WIP: Close private D-BUS connections

This fixes the message, but I have a feeling that this might not be sufficient. a87b5a1's commit message indicates that sometimes NM actually uses shared connections, in which case we mustn't close them?
Comment 2 Dan Williams 2013-04-30 21:17:25 UTC
The private server stuff will never be a shared connection actually so I think the patch is correct.  Need more info from people who know dbus better though...
Comment 3 Colin Walters 2013-04-30 21:44:16 UTC
Review of attachment 242197 [details] [review]:

My memory of libdbus is fairly dim now, but the patch looks reasonable to me.  It'd probably be cleaner if we could arrange for dbus_connection_close() to be explicitly called exactly once in the right place, but eh.
Comment 4 Dan Williams 2013-05-03 15:43:54 UTC
Looking at the dbus code for DBusServer instances quickly, on the *server* side I believe all the sockets are non-shared.  Only in on the client side are they shared.  So I think the patch is correct.
Comment 5 Dan Williams 2013-05-03 15:49:31 UTC
Pushed to master with a small fixup; we can remove the dbus_connection_close() in the Disconnected handle because the connection will be removed from the hash table, thus closing it too.  Thanks!