[lxc-devel] [PATCH] cgmanager: avoid stray dbus connections

[Stéphane Graber]

On Mon, Mar 10, 2014 at 09:41:34PM -0500, Serge Hallyn wrote:
> There are two parts to this fix.
> 
> First, create a private DBusConnection manually, instead of using
> nih_dbus_connect.  The latter always creates a shared connection,
> which cannot be closed.  Note: creating an actual shared connection,
> mutexing it among all threads, and creating per-thread proxies would
> be an alternative - however we don't want long-lived connections as
> they tend not to be reliable (especially if cgmanager restarts).
> 
> Second, use pthread_setspecific to create per-thread keys which can
> be associated with destructors.  Specify a destructor which closes
> the dbus connection.  If a thread dies while holding cgmanager,
> the connection will be closed.  Otherwise, we close the connection
> and unset the key.
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>

Acked-by: Stéphane Graber <stgraber at ubuntu.com>

> ---
>  src/lxc/cgmanager.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index c80a136..4cf6397 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -70,10 +70,42 @@ struct cgm_data {
>  static __thread NihDBusProxy *cgroup_manager = NULL;
>  static __thread DBusConnection *connection = NULL;
>  static __thread bool cgm_keep_connection = false;
> +
> +
> +struct cgm_key_t {
> +	DBusConnection *connection;
> +	NihDBusProxy *cgmanager;
> +} cgm_key;
> +
> +void destructor(void *arg) {
> +	struct cgm_key_t *cgm_key = arg;
> +	nih_free(cgm_key->cgmanager);
> +	dbus_connection_flush(cgm_key->connection);
> +	dbus_connection_close(cgm_key->connection);
> +	dbus_connection_unref(cgm_key->connection);
> +}
> +static pthread_key_t key;
> +static void make_key() {
> +	pthread_key_create(&key, destructor);
> +}
> +
> +static void init_cgm_destructor(DBusConnection *c, NihDBusProxy *cgm)
> +{
> +	static pthread_once_t key_once = PTHREAD_ONCE_INIT;
> +	pthread_once(&key_once, make_key);
> +	cgm_key.connection = c;
> +	cgm_key.cgmanager = cgm;
> +	if (!cgm)
> +		pthread_setspecific(key, NULL);
> +	else if (pthread_getspecific(key) == NULL)
> +		pthread_setspecific(key, &cgm_key);
> +}
> +
>  #else
>  static NihDBusProxy *cgroup_manager = NULL;
>  static DBusConnection *connection = NULL;
>  static bool cgm_keep_connection = false;
> +static inline void init_cgm_destructor(DBusConnection *c, NihDBusProxy *cgm) { }
>  #endif
>  
>  static struct cgroup_ops cgmanager_ops;
> @@ -87,9 +119,13 @@ static void cgm_dbus_disconnect(void)
>  	if (cgroup_manager)
>  		nih_free(cgroup_manager);
>  	cgroup_manager = NULL;
> -	if (connection)
> +	if (connection) {
> +		dbus_connection_flush(connection);
> +		dbus_connection_close(connection);
>  		dbus_connection_unref(connection);
> +	}
>  	connection = NULL;
> +	init_cgm_destructor(NULL, NULL);
>  }
>  
>  #define CGMANAGER_DBUS_SOCK "unix:path=/sys/fs/cgroup/cgmanager/sock"
> @@ -105,14 +141,22 @@ static bool do_cgm_dbus_connect(void)
>  
>  	dbus_error_init(&dbus_error);
>  
> -	connection = nih_dbus_connect(CGMANAGER_DBUS_SOCK, NULL);
> +	connection = dbus_connection_open_private(CGMANAGER_DBUS_SOCK, &dbus_error);
>  	if (!connection) {
> +		ERROR("Failed opening dbus connection: %s: %s",
> +				dbus_error.name, dbus_error.message);
> +		dbus_error_free(&dbus_error);
> +		return false;
> +	}
> +	if (nih_dbus_setup(connection, NULL) < 0) {
>  		NihError *nerr;
>  		nerr = nih_error_get();
>  		DEBUG("Unable to open cgmanager connection at %s: %s", CGMANAGER_DBUS_SOCK,
>  			nerr->message);
>  		nih_free(nerr);
>  		dbus_error_free(&dbus_error);
> +		dbus_connection_unref(connection);
> +		connection = NULL;
>  		return false;
>  	}
>  	dbus_connection_set_exit_on_disconnect(connection, FALSE);
> @@ -138,6 +182,7 @@ static bool do_cgm_dbus_connect(void)
>  		cgm_dbus_disconnect();
>  		return false;
>  	}
> +	init_cgm_destructor(connection, cgroup_manager);
>  	return true;
>  }
>  
> -- 
> 1.9.0
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140311/6bbc9995/attachment.pgp>