[cgmanager-devel] [PATCH 1/1] add timeout to connections

Serge Hallyn serge.hallyn at ubuntu.com
Thu Apr 3 23:53:48 UTC 2014


To avoid having lazy callers fill up our fdtable.  Every new method that
is called causes the timeout to be reset.

In order to avoid having to walk all of the nih_timers to look for the
one to reset, we keep our own separate hash of connection timers keyed
on the connection itself.  This is the bulk of the new code.

Also increase our open file limit to 10k.

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 Makefile.am          |  10 ++-
 cgmanager-proxy.c    |   2 +
 cgmanager.c          |  10 +++
 frontend.c           | 129 +++++++++++++++++++++++++++++++++++++++
 frontend.h           |   1 +
 tests/test-timeout.c | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 320 insertions(+), 1 deletion(-)
 create mode 100644 tests/test-timeout.c

diff --git a/Makefile.am b/Makefile.am
index b552809..7cd5ece 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -112,4 +112,12 @@ TESTS_CGM_CONCURRENT: tests/cgm-concurrent.o
 	$(CCLD) -o tests/cgm-concurrent tests/cgm-concurrent.o \
 		$(NIH_LIBS) $(NIH_DBUS_LIBS) $(DBUS_LIBS) -lpthread -lcgmanager
 
-tests: TESTS_CGM_CONCURRENT
+tests/test-timeout.o: tests/test-timeout.c
+	$(CC) -I. $(NIH_CFLAGS) $(NIH_DBUS_CFLAGS) $(DBUS_CFLAGS)  -c \
+		-fPIC -DPIC -o tests/test-timeout.o tests/test-timeout.c
+
+TESTS_CGM_TIMEOUT: tests/test-timeout.o
+	$(CCLD) -o tests/test-timeout tests/test-timeout.o \
+		$(NIH_LIBS) $(NIH_DBUS_LIBS) $(DBUS_LIBS) -lcgmanager
+
+tests: TESTS_CGM_CONCURRENT TESTS_CGM_TIMEOUT
diff --git a/cgmanager-proxy.c b/cgmanager-proxy.c
index 1876e16..6de01e2 100644
--- a/cgmanager-proxy.c
+++ b/cgmanager-proxy.c
@@ -1006,6 +1006,8 @@ main (int argc, char *argv[])
 		setns_user_supported = true;
 	}
 
+	connection_timeout_init();
+
 	/* Become daemon */
 	if (daemonise) {
 		if (nih_main_daemonise () < 0) {
diff --git a/cgmanager.c b/cgmanager.c
index 880c4fb..1626ba1 100644
--- a/cgmanager.c
+++ b/cgmanager.c
@@ -18,6 +18,7 @@
  */
 
 #include <frontend.h>
+#include <sys/resource.h>
 
 /*
  * Maximum depth of directories we allow in Create
@@ -814,6 +815,7 @@ main (int argc, char *argv[])
 	int		ret;
 	DBusServer *	server;
 	struct stat sb;
+	struct rlimit newrlimit;
 
 	nih_main_init (argv[0]);
 
@@ -870,6 +872,14 @@ main (int argc, char *argv[])
 		setns_user_supported = true;
 	}
 
+	connection_timeout_init();
+
+	newrlimit.rlim_cur = 10000;
+	newrlimit.rlim_max = 10000;
+	if (setrlimit(RLIMIT_NOFILE, &newrlimit) < 0)
+		nih_warn("Failed to increase open file limit: %s",
+			strerror(errno));
+
 	/* Become daemon */
 	if (daemonise) {
 		if (nih_main_daemonise () < 0) {
diff --git a/frontend.c b/frontend.c
index 0190f5a..44e15be 100644
--- a/frontend.c
+++ b/frontend.c
@@ -22,6 +22,9 @@
 
 #define __frontend_c
 #include <frontend.h>
+#include <nih/hash.h>
+#include <nih/list.h>
+#include <nih/timer.h>
 
 int daemonise = FALSE;
 bool setns_pid_supported = false;
@@ -29,6 +32,99 @@ unsigned long mypidns;
 bool setns_user_supported = false;
 unsigned long myuserns;
 
+/* functions to time out connections */
+
+typedef struct timer_hash_entry {
+	NihList entry;
+	DBusConnection *conn;
+	NihTimer *timer;
+} timer_hash_entry;
+
+static NihHash *timer_hash;
+
+static const void *timer_key_fn(NihList *entry)
+{
+	timer_hash_entry *e = (timer_hash_entry *)entry;
+	return e->conn;
+}
+
+static uint32_t timer_hash_fn(const void *conn)
+{
+	unsigned long val = (unsigned long)conn;
+
+	if (sizeof(conn) > 4)
+		val ^= (val >> 32);
+	return (uint32_t)val;
+}
+
+static int timer_cmp_fn(const void *v1, const void *v2)
+{
+	if (v1 == v2)
+		return 0;
+	return 1;
+}
+
+static void timeout_remove(DBusConnection *conn)
+{
+	timer_hash_entry *t;
+	NihTimer *timer;
+
+	t = (timer_hash_entry *) nih_hash_lookup(timer_hash, conn);
+	if (!t)
+		return;
+	nih_list_remove(&t->entry);
+	timer = t->timer;
+	nih_list_remove(&timer->entry);
+	nih_free(timer);
+	nih_free(t);
+}
+
+static void timeout_handler(void *data, NihTimer *timer)
+{
+	timer_hash_entry *t = (timer_hash_entry *)data;
+	DBusConnection *conn = t->conn;
+
+	dbus_connection_close(conn);
+	dbus_connection_unref(conn);
+	/* nih will take care of the NihTimer itself */
+	nih_list_remove(&t->entry);
+	nih_free(t);
+}
+
+static void timeout_add(DBusConnection *);
+
+static void timeout_reset(DBusConnection *conn)
+{
+	timer_hash_entry *t; /* entry into our own timer_hash */
+	NihTimer *timer;
+
+	t = (timer_hash_entry *) nih_hash_lookup(timer_hash, conn);
+	if (!t)
+		return;
+	nih_list_remove(&t->entry);
+	timer = t->timer;
+	nih_list_remove(&timer->entry);
+	nih_free(timer);
+	nih_free(t);
+
+	timeout_add(conn);
+}
+
+static void timeout_add(DBusConnection *conn)
+{
+	timer_hash_entry *t;
+	NihTimer *timer;
+
+	t = NIH_MUST( nih_new(NULL, timer_hash_entry) );
+	timer = NIH_MUST( nih_timer_add_timeout(NULL, 20,
+		timeout_handler, t) );
+	t->conn = conn;
+	t->timer = timer;
+	nih_list_init((NihList *) t);
+	nih_hash_add(timer_hash, (NihList *)t);
+}
+
+/* reject unsafe cgroups */
 bool sane_cgroup(const char *cgroup)
 {
 	if (!cgroup)
@@ -238,6 +334,7 @@ int cgmanager_get_pid_cgroup_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_GET_PID);
 	if (!d)
 		return -1;
@@ -280,6 +377,7 @@ int cgmanager_get_pid_cgroup (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -337,6 +435,7 @@ int cgmanager_move_pid_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_MOVE_PID);
 	if (!d)
 		return -1;
@@ -379,6 +478,7 @@ int cgmanager_move_pid (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -430,6 +530,7 @@ int cgmanager_move_pid_abs_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_MOVE_PID_ABS);
 	if (!d)
 		return -1;
@@ -467,6 +568,7 @@ int cgmanager_move_pid_abs (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -550,6 +652,7 @@ int cgmanager_create_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_CREATE);
 	if (!d)
 		return -1;
@@ -594,6 +697,7 @@ int cgmanager_create (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 				"Could not get client socket.");
@@ -635,6 +739,7 @@ int cgmanager_chown_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_CHOWN);
 	if (!d)
 		return -1;
@@ -682,6 +787,7 @@ int cgmanager_chown (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -740,6 +846,7 @@ int cgmanager_chmod_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_CHMOD);
 	if (!d)
 		return -1;
@@ -784,6 +891,7 @@ int cgmanager_chmod (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -828,6 +936,7 @@ int cgmanager_get_value_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_GET_VALUE);
 	if (!d)
 		return -1;
@@ -879,6 +988,7 @@ int cgmanager_get_value (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -919,6 +1029,7 @@ int cgmanager_set_value_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_SET_VALUE);
 	if (!d)
 		return -1;
@@ -967,6 +1078,7 @@ int cgmanager_set_value (void *data, NihDBusMessage *message,
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -1010,6 +1122,7 @@ int cgmanager_remove_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_REMOVE);
 	if (!d)
 		return -1;
@@ -1054,6 +1167,7 @@ int cgmanager_remove (void *data, NihDBusMessage *message, const char *controlle
 			"message was null");
 		return -1;
 	}
+	timeout_reset(message->connection);
 
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
@@ -1111,6 +1225,7 @@ int cgmanager_get_tasks_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_GET_TASKS);
 	if (!d)
 		return -1;
@@ -1154,6 +1269,7 @@ int cgmanager_get_tasks (void *data, NihDBusMessage *message, const char *contro
 		return -1;
 	}
 
+	timeout_reset(message->connection);
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
 					     "Could not get client socket.");
@@ -1238,6 +1354,7 @@ int cgmanager_list_children_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_LIST_CHILDREN);
 	if (!d)
 		return -1;
@@ -1281,6 +1398,7 @@ int cgmanager_list_children (void *data, NihDBusMessage *message,
 			"message was null");
 		return -1;
 	}
+	timeout_reset(message->connection);
 
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
@@ -1324,6 +1442,7 @@ int cgmanager_remove_on_empty_scm (void *data, NihDBusMessage *message,
 {
 	struct scm_sock_data *d;
 
+	timeout_reset(message->connection);
 	d = alloc_scm_sock_data(message, sockfd, REQ_TYPE_REMOVE_ON_EMPTY);
 	if (!d)
 		return -1;
@@ -1366,6 +1485,7 @@ int cgmanager_remove_on_empty (void *data, NihDBusMessage *message,
 			"message was null");
 		return -1;
 	}
+	timeout_reset(message->connection);
 
 	if (!dbus_connection_get_socket(message->connection, &fd)) {
 		nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS,
@@ -1396,6 +1516,7 @@ int cgmanager_remove_on_empty (void *data, NihDBusMessage *message,
 int
 cgmanager_get_api_version(void *data, NihDBusMessage *message, int *version)
 {
+	timeout_reset(message->connection);
 	nih_assert(message);
 	nih_assert(version);
 	*version = API_VERSION;
@@ -1423,6 +1544,7 @@ int client_connect (DBusServer *server, DBusConnection *conn)
 				"/org/linuxcontainers/cgmanager",
 				cgmanager_interfaces, NULL));
 
+	timeout_add(conn);
 	return TRUE;
 }
 
@@ -1431,5 +1553,12 @@ void client_disconnect (DBusConnection *conn)
 	if (conn == NULL)
 		return;
 
+	timeout_remove(conn);
 	nih_info (_("Disconnected from private client"));
 }
+
+void connection_timeout_init(void)
+{
+	timer_hash = NIH_MUST( nih_hash_new(NULL, 1024,
+			timer_key_fn, timer_hash_fn, timer_cmp_fn) );
+}
diff --git a/frontend.h b/frontend.h
index 0c6c1f6..431cd62 100644
--- a/frontend.h
+++ b/frontend.h
@@ -150,6 +150,7 @@ int cgmanager_ping (void *data, NihDBusMessage *message, int junk);
 
 int client_connect (DBusServer *server, DBusConnection *conn);
 void client_disconnect (DBusConnection *conn);
+void connection_timeout_init(void);
 
 bool sane_cgroup(const char *cgroup);
 
diff --git a/tests/test-timeout.c b/tests/test-timeout.c
new file mode 100644
index 0000000..2c9be61
--- /dev/null
+++ b/tests/test-timeout.c
@@ -0,0 +1,169 @@
+/* concurrent.c
+ *
+ * Copyright © 2013 S.Çağlar Onur <caglar at 10ur.org>
+ * Copyright © 2014 Serge Hallyn <serge.hallyn at ubuntu.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * compile with
+gcc -o cgm-concurrent cgm-concurrent.c -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/cgmanager -lcgmanager -lnih -lnih-dbus -ldbus-1 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnux/dbus-1.0/include -ldbus-1 -lpthread -g XXX
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <dirent.h>
+#include <fcntl.h>
+#include <ctype.h>
+#include <pthread.h>
+#include <grp.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/param.h>
+#include <sys/inotify.h>
+#include <sys/mount.h>
+#include <netinet/in.h>
+#include <net/if.h>
+#include <sys/syscall.h>
+
+#include <limits.h>
+#include <malloc.h>
+#include <getopt.h>
+#include <stdlib.h>
+
+#include <nih-dbus/dbus_connection.h>
+#include <cgmanager/cgmanager-client.h>
+#include <nih/alloc.h>
+#include <nih/error.h>
+#include <nih/string.h>
+#include <stdbool.h>
+#include <cgmanager-client.h>
+
+NihDBusProxy *cgroup_manager = NULL;
+DBusConnection *connection = NULL;
+
+static const struct option options[] = {
+	{ "threads",     required_argument, NULL, 'j' },
+	{ "iterations",  required_argument, NULL, 'i' },
+	{ "connect-only",  no_argument, NULL, 'c' },
+	{ 0, 0, 0, 0 },
+};
+
+void cgm_dbus_disconnected(DBusConnection *connection)
+{
+	cgroup_manager = NULL;
+	connection = NULL;
+}
+
+static void cgm_dbus_disconnect(void)
+{
+	if (cgroup_manager)
+		nih_free(cgroup_manager);
+	cgroup_manager = NULL;
+	if (connection) {
+		dbus_connection_flush(connection);
+		dbus_connection_close(connection);
+		dbus_connection_unref(connection);
+	}
+	connection = NULL;
+}
+
+#define CGMANAGER_DBUS_SOCK "unix:path=/sys/fs/cgroup/cgmanager/sock"
+static bool cgm_dbus_connect(void)
+{
+	DBusError dbus_error;
+
+	dbus_error_init(&dbus_error);
+
+	connection = nih_dbus_connect(CGMANAGER_DBUS_SOCK, cgm_dbus_disconnected);
+	if (!connection) {
+		dbus_error_free(&dbus_error);
+		return false;
+	}
+	dbus_connection_set_exit_on_disconnect(connection, FALSE);
+	dbus_error_free(&dbus_error);
+	cgroup_manager = nih_dbus_proxy_new(NULL, connection,
+				NULL /* p2p */,
+				"/org/linuxcontainers/cgmanager", NULL, NULL);
+	if (!cgroup_manager) {
+		NihError *nerr;
+		nerr = nih_error_get();
+		nih_free(nerr);
+		cgm_dbus_disconnect();
+		return false;
+	}
+
+	// force fd passing negotiation
+	if (cgmanager_ping_sync(NULL, cgroup_manager, 0) != 0) {
+		NihError *nerr;
+		nerr = nih_error_get();
+		nih_free(nerr);
+		cgm_dbus_disconnect();
+		return false;
+	}
+	return true;
+}
+
+int main(int argc, char *argv[]) {
+
+	int32_t v;
+	NihError *nerr;
+
+	if (!cgm_dbus_connect()) {
+		fprintf(stderr, "Error connecting to dbus\n");
+		exit(1);
+	}
+
+	sleep(30);
+	if (cgmanager_ping_sync(NULL, cgroup_manager, v) == 0) {
+		fprintf(stderr, "connection did not time out");
+		exit(1);
+	}
+	nerr = nih_error_get();
+	nih_free(nerr);
+
+	/* Now open a connection, wait 15 seconds, do a request,
+	 * wait another 15 seconds - connection should still be open;
+	 * wait anothe 15 seconds, it should be closed.
+	 */
+	if (!cgm_dbus_connect()) {
+		fprintf(stderr, "Error connecting to dbus\n");
+		exit(1);
+	}
+	sleep(15);
+	if (cgmanager_get_api_version_sync(NULL, cgroup_manager, &v) != 0) {
+		nerr = nih_error_get();
+		nih_free(nerr);
+		fprintf(stderr, "Error creating freezer cgroup\n");
+		exit(1);
+	}
+	sleep(15);
+	if (cgmanager_ping_sync(NULL, cgroup_manager, v) != 0) {
+		nerr = nih_error_get();
+		nih_free(nerr);
+		fprintf(stderr, "timeout was not reset");
+		exit(1);
+	}
+	sleep(30);
+	if (cgmanager_ping_sync(NULL, cgroup_manager, v) == 0) {
+		fprintf(stderr, "connection did not time out after a reset");
+		exit(1);
+	}
+	nerr = nih_error_get();
+	nih_free(nerr);
+	fprintf(stderr, "all tests passed\n");
+	exit(0);
+}
-- 
1.9.1



More information about the cgmanager-devel mailing list