[PATCH 0/3] Add DBus disconnect watch support

[PATCH 0/3] Add DBus disconnect watch support

[Jukka Rissanen]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

Hi,

this set allows user to monitor the disconnect status
of a service.

Cheers,
Jukka

Jukka Rissanen (3):
  dbus: Add AddMatch and RemoveMatch support
  dbus: Add disconnect watch support
  unit: dbus: Add test for disconnect watch API

 Makefile.am            |   4 +
 ell/dbus-watch.c       | 267 ++++++++++++++++++++++++++++++++++
 ell/dbus.c             |  26 ++++
 ell/dbus.h             |  21 +++
 unit/test-dbus-watch.c | 388 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 706 insertions(+)
 create mode 100644 ell/dbus-watch.c
 create mode 100644 unit/test-dbus-watch.c

-- 
1.8.3.1

[PATCH 1/3] dbus: Add AddMatch and RemoveMatch support

[Jukka Rissanen]

[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]

We do not check for errors as they are typically unrecoverable
in this case and in most cases ignored anyway.

This is the comment from libdbus API documentation for
dbus_bus_add_match() about error checking:

"If you pass NULL for the error, this function will not block;
the match thus won't be added until you flush the connection,
and if there's an error adding the match you won't find out
about it. This is generally acceptable, since the possible
errors (including a lack of resources in the bus, the connection
having exceeded its quota of active match rules, or the match
rule being unparseable) are generally unrecoverable."
---
 ell/dbus.c | 26 ++++++++++++++++++++++++++
 ell/dbus.h |  4 ++++
 2 files changed, 30 insertions(+)

diff --git a/ell/dbus.c b/ell/dbus.c
index 26456ac..f115584 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1222,3 +1222,29 @@ LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
 
 	return _dbus_object_tree_unregister(dbus->tree, path, interface);
 }
+
+static void send_match(struct l_dbus *dbus, const char *rule,
+						const char *method)
+{
+	struct l_dbus_message *message;
+
+	message = l_dbus_message_new_method_call(dbus,
+						DBUS_SERVICE_DBUS,
+						DBUS_PATH_DBUS,
+						DBUS_INTERFACE_DBUS,
+						method);
+
+	l_dbus_message_set_arguments(message, "s", rule);
+
+	send_message(dbus, false, message, NULL, NULL, NULL);
+}
+
+LIB_EXPORT void l_dbus_bus_add_match(struct l_dbus *dbus, const char *rule)
+{
+	send_match(dbus, rule, "AddMatch");
+}
+
+LIB_EXPORT void l_dbus_bus_remove_match(struct l_dbus *dbus, const char *rule)
+{
+	send_match(dbus, rule, "RemoveMatch");
+}
diff --git a/ell/dbus.h b/ell/dbus.h
index 7622b47..9fcce03 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -200,6 +200,10 @@ bool l_dbus_register_interface(struct l_dbus *dbus,
 				l_dbus_destroy_func_t destroy);
 bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
 					const char *interface);
+
+void l_dbus_bus_add_match(struct l_dbus *dbus, const char *rule);
+void l_dbus_bus_remove_match(struct l_dbus *dbus, const char *rule);
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1

[PATCH 2/3] dbus: Add disconnect watch support

[Jukka Rissanen]

[-- Attachment #1: Type: text/plain, Size: 9370 bytes --]

Allow caller to monitor the disconnect status of a service.
---
 Makefile.am      |   1 +
 ell/dbus-watch.c | 267 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus.h       |  17 ++++
 3 files changed, 285 insertions(+)
 create mode 100644 ell/dbus-watch.c

diff --git a/Makefile.am b/Makefile.am
index 50f81eb..3dcc7ee 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -63,6 +63,7 @@ ell_libell_la_SOURCES = $(linux_headers) \
 			ell/dbus-message.c \
 			ell/dbus-util.c \
 			ell/dbus-service.c \
+			ell/dbus-watch.c \
 			ell/gvariant-private.h \
 			ell/gvariant-util.c \
 			ell/siphash-private.h \
diff --git a/ell/dbus-watch.c b/ell/dbus-watch.c
new file mode 100644
index 0000000..005bc83
--- /dev/null
+++ b/ell/dbus-watch.c
@@ -0,0 +1,267 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2015  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library 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
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdarg.h>
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+
+#include "log.h"
+#include "util.h"
+#include "queue.h"
+#include "string.h"
+#include "dbus.h"
+#include "dbus-private.h"
+#include "private.h"
+
+struct filter_data {
+	struct l_dbus *dbus;
+	l_dbus_message_func_t handle_func;
+	l_dbus_watch_func_t connect_func;
+	l_dbus_watch_func_t disconnect_func;
+	char *name;
+	char *owner;
+	char *path;
+	char *interface;
+	char *member;
+	char *argument;
+	void *user_data;
+	l_dbus_destroy_func_t destroy_func;
+};
+
+static void format_rule(struct filter_data *data, char *rule, size_t size)
+{
+	const char *sender;
+	int offset;
+
+	offset = snprintf(rule, size, "type='signal'");
+	sender = data->name ? : data->owner;
+
+	if (sender)
+		offset += snprintf(rule + offset, size - offset,
+				",sender='%s'", sender);
+	if (data->path)
+		offset += snprintf(rule + offset, size - offset,
+				",path='%s'", data->path);
+	if (data->interface)
+		offset += snprintf(rule + offset, size - offset,
+				",interface='%s'", data->interface);
+	if (data->member)
+		offset += snprintf(rule + offset, size - offset,
+				",member='%s'", data->member);
+	if (data->argument)
+		snprintf(rule + offset, size - offset,
+				",arg0='%s'", data->argument);
+}
+
+static void add_match(struct filter_data *data, l_dbus_message_func_t filter)
+{
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	format_rule(data, rule, sizeof(rule));
+
+	l_dbus_bus_add_match(data->dbus, rule);
+
+	data->handle_func = filter;
+}
+
+static void remove_match(struct filter_data *data)
+{
+	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
+
+	format_rule(data, rule, sizeof(rule));
+
+	l_dbus_bus_remove_match(data->dbus, rule);
+}
+
+static struct filter_data *filter_data_get(struct l_dbus *dbus,
+					l_dbus_message_func_t filter,
+					const char *sender,
+					const char *path,
+					const char *interface,
+					const char *member,
+					const char *argument,
+					l_dbus_watch_func_t connect_func,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy)
+{
+	struct filter_data *data;
+	const char *name = NULL, *owner = NULL;
+
+	if (sender) {
+		if (sender[0] == ':')
+			owner = sender;
+		else
+			name = sender;
+	}
+
+	data = l_new(struct filter_data, 1);
+
+	data->dbus = dbus;
+	data->handle_func = filter;
+	data->connect_func = connect_func;
+	data->disconnect_func = disconnect_func;
+	data->name = l_strdup(name);
+	data->owner = l_strdup(owner);
+	data->path = l_strdup(path);
+	data->interface = l_strdup(interface);
+	data->member = l_strdup(member);
+	data->argument = l_strdup(argument);
+	data->user_data = user_data;
+	data->destroy_func = destroy;
+
+	add_match(data, filter);
+
+	return data;
+}
+
+static void filter_data_destroy(void *user_data)
+{
+	struct filter_data *data = user_data;
+
+	remove_match(data);
+
+	l_free(data->name);
+	l_free(data->owner);
+	l_free(data->path);
+	l_free(data->interface);
+	l_free(data->member);
+	l_free(data->argument);
+
+	if (data->destroy_func)
+		data->destroy_func(data->user_data);
+
+	l_free(data);
+}
+
+static void message_filter(struct l_dbus_message *message, void *user_data)
+{
+	struct filter_data *data = user_data;
+	const char *sender, *path, *iface, *member, *arg = NULL;
+
+	if (_dbus_message_get_type(message) != DBUS_MESSAGE_TYPE_SIGNAL)
+		return;
+
+	sender = l_dbus_message_get_sender(message);
+	if (!sender)
+		return;
+
+	path = l_dbus_message_get_path(message);
+	iface = l_dbus_message_get_interface(message);
+	member = l_dbus_message_get_member(message);
+
+	l_dbus_message_get_arguments(message, "s", &arg);
+
+	if (data->owner && strcmp(sender, data->owner))
+		goto out;
+
+	if (data->path && strcmp(path, data->path))
+		goto out;
+
+	if (data->interface && strcmp(iface, data->interface))
+		goto out;
+
+	if (data->member && strcmp(member, data->member))
+		goto out;
+
+	if (arg && data->argument && strcmp(arg, data->argument))
+		goto out;
+
+	if (data->handle_func)
+		data->handle_func(message, data);
+
+out:
+	return;
+}
+
+static void service_filter(struct l_dbus_message *message, void *user_data)
+{
+	struct filter_data *data = user_data;
+	char *name, *old, *new;
+
+	if (!l_dbus_message_get_arguments(message, "sss",
+						&name, &old, &new)) {
+		l_error("Invalid arguments for NameOwnerChanged signal");
+		return;
+	}
+
+	if (*new == '\0') {
+		if (data->disconnect_func)
+			data->disconnect_func(data->dbus, data->user_data);
+	} else {
+		if (data->connect_func)
+			data->connect_func(data->dbus, data->user_data);
+	}
+}
+
+LIB_EXPORT unsigned int l_dbus_add_service_watch(struct l_dbus *dbus,
+					const char *name,
+					l_dbus_watch_func_t connect_func,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy)
+{
+	struct filter_data *data;
+
+	if (!name)
+		return 0;
+
+	if (connect_func) {
+		l_error("Connect watch not yet supported");
+		return -EOPNOTSUPP;
+	}
+
+	data = filter_data_get(dbus, service_filter,
+				DBUS_SERVICE_DBUS, DBUS_PATH_DBUS,
+				DBUS_INTERFACE_DBUS, "NameOwnerChanged",
+				name,
+				connect_func,
+				disconnect_func,
+				user_data,
+				destroy);
+	if (!data)
+		return 0;
+
+	return l_dbus_register(dbus, message_filter, data,
+							filter_data_destroy);
+}
+
+LIB_EXPORT unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
+					const char *name,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy)
+{
+	return l_dbus_add_service_watch(dbus, name, NULL, disconnect_func,
+							user_data, destroy);
+}
+
+LIB_EXPORT bool l_dbus_remove_watch(struct l_dbus *dbus, unsigned int id)
+{
+	return l_dbus_unregister(dbus, id);
+}
diff --git a/ell/dbus.h b/ell/dbus.h
index 9fcce03..8952dcf 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -36,6 +36,8 @@ extern "C" {
 #define DBUS_PATH_DBUS		"/org/freedesktop/DBus"
 #define DBUS_INTERFACE_DBUS	"org.freedesktop.DBus"
 
+#define DBUS_MAXIMUM_MATCH_RULE_LENGTH	1024
+
 enum l_dbus_bus {
 	L_DBUS_SYSTEM_BUS,
 	L_DBUS_SESSION_BUS,
@@ -52,6 +54,8 @@ typedef void (*l_dbus_debug_func_t) (const char *str, void *user_data);
 typedef void (*l_dbus_destroy_func_t) (void *user_data);
 typedef void (*l_dbus_interface_setup_func_t) (struct l_dbus_interface *);
 
+typedef void (*l_dbus_watch_func_t) (struct l_dbus *dbus, void *user_data);
+
 struct l_dbus *l_dbus_new(const char *address);
 struct l_dbus *l_dbus_new_default(enum l_dbus_bus bus);
 void l_dbus_destroy(struct l_dbus *dbus);
@@ -204,6 +208,19 @@ bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
 void l_dbus_bus_add_match(struct l_dbus *dbus, const char *rule);
 void l_dbus_bus_remove_match(struct l_dbus *dbus, const char *rule);
 
+unsigned int l_dbus_add_service_watch(struct l_dbus *dbus,
+					const char *name,
+					l_dbus_watch_func_t connect_func,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy);
+unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
+					const char *name,
+					l_dbus_watch_func_t disconnect_func,
+					void *user_data,
+					l_dbus_destroy_func_t destroy);
+bool l_dbus_remove_watch(struct l_dbus *dbus, unsigned int id);
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1

[PATCH 3/3] unit: dbus: Add test for disconnect watch API

[Jukka Rissanen]

[-- Attachment #1: Type: text/plain, Size: 9469 bytes --]

Test the dbus disconnect watch support.
---
 Makefile.am            |   3 +
 unit/test-dbus-watch.c | 388 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 391 insertions(+)
 create mode 100644 unit/test-dbus-watch.c

diff --git a/Makefile.am b/Makefile.am
index 3dcc7ee..3b6017c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -103,6 +103,7 @@ unit_tests = unit/test-unit \
 			unit/test-dbus-util \
 			unit/test-dbus-message \
 			unit/test-dbus-service \
+			unit/test-dbus-watch \
 			unit/test-gvariant-util \
 			unit/test-gvariant-message \
 			unit/test-siphash \
@@ -148,6 +149,8 @@ unit_test_dbus_util_LDADD = ell/libell-private.la
 
 unit_test_dbus_service_LDADD = ell/libell-private.la
 
+unit_test_dbus_watch_LDADD = ell/libell-private.la
+
 unit_test_gvariant_util_LDADD = ell/libell-private.la
 
 unit_test_gvariant_message_LDADD = ell/libell-private.la
diff --git a/unit/test-dbus-watch.c b/unit/test-dbus-watch.c
new file mode 100644
index 0000000..6c93a87
--- /dev/null
+++ b/unit/test-dbus-watch.c
@@ -0,0 +1,388 @@
+/*
+ *
+ *  Embedded Linux library
+ *
+ *  Copyright (C) 2011-2015  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library 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
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <sys/wait.h>
+#include <assert.h>
+#include <time.h>
+
+#include <ell/ell.h>
+
+#define TEST_BUS_ADDRESS "unix:path=/tmp/ell-watch-test-bus"
+
+#define SERVICE_TO_WATCH "org.test.watch.service"
+#define SERVICE_MONITOR "org.test.main.service"
+
+static pid_t dbus_daemon_pid = -1;
+static pid_t test_daemon_pid = -1;
+
+static struct l_timeout *timeout = NULL;
+static bool failure = false;
+static struct tm *tm = NULL;
+
+static pid_t launch(char *name, char **argv, char **envp)
+{
+	pid_t pid;
+
+	l_info("launching %s", name);
+
+	pid = fork();
+	if (pid < 0) {
+		l_error("failed to fork new process");
+		return -1;
+	}
+
+	if (pid == 0) {
+		execve(argv[0], argv, envp);
+		exit(EXIT_SUCCESS);
+	}
+
+	l_info("%s process %d created", name, pid);
+
+	return pid;
+}
+
+static void start_dbus_daemon(void)
+{
+	char *prg_argv[6];
+	char *prg_envp[1];
+
+	prg_argv[0] = "/usr/bin/dbus-daemon";
+	prg_argv[1] = "--session";
+	prg_argv[2] = "--address=" TEST_BUS_ADDRESS;
+	prg_argv[3] = "--nopidfile";
+	prg_argv[4] = "--nofork";
+	prg_argv[5] = NULL;
+
+	prg_envp[0] = NULL;
+
+	dbus_daemon_pid = launch("dbus-daemon",
+				prg_argv, prg_envp);
+}
+
+static void start_test_daemon(char *argv0, char *argv1)
+{
+	char *prg_argv[3];
+	char *prg_envp[1];
+
+	prg_argv[0] = argv0;
+	prg_argv[1] = argv1;
+	prg_argv[2] = NULL;
+
+	prg_envp[0] = NULL;
+
+	test_daemon_pid = launch("test daemon",
+				prg_argv, prg_envp);
+}
+
+static void signal_handler(struct l_signal *signal, uint32_t signo,
+							void *user_data)
+{
+	switch (signo) {
+	case SIGINT:
+	case SIGTERM:
+		l_info("Terminate");
+		l_main_quit();
+		break;
+	case SIGCHLD:
+		while (1) {
+			pid_t pid;
+			int status;
+
+			pid = waitpid(WAIT_ANY, &status, WNOHANG);
+			if (pid < 0 || pid == 0)
+				break;
+
+			l_info("process %d terminated with status=%d\n",
+								pid, status);
+
+			if (pid == dbus_daemon_pid) {
+				dbus_daemon_pid = -1;
+				l_main_quit();
+			}
+		}
+		break;
+	}
+}
+
+static void ready_callback(void *user_data)
+{
+	char *argv0 = user_data;
+
+	l_info("test app ready");
+
+	start_test_daemon(argv0, "test");
+}
+
+static void ready_tester(void *user_data)
+{
+	l_info("tester ready");
+
+	return;
+}
+
+struct data {
+	char *name;
+	struct l_dbus *dbus;
+};
+
+static void release_name_setup(struct l_dbus_message *message, void *user_data)
+{
+	struct data *data = user_data;
+
+	l_dbus_message_set_arguments(message, "s", data->name);
+}
+
+static void release_name_callback(struct l_dbus_message *message,
+							void *user_data)
+{
+	const char *error, *text;
+	uint32_t result;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		goto done;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "u", &result))
+		goto done;
+
+	l_info("release name result = %d", result);
+
+done:
+	l_main_quit();
+}
+
+static void request_name_setup(struct l_dbus_message *message,
+				void *user_data)
+{
+	struct data *data = user_data;
+
+	l_dbus_message_set_arguments(message, "su", data->name, 0);
+}
+
+static void request_name_callback(struct l_dbus_message *message,
+							void *user_data)
+{
+	const char *error, *text;
+	uint32_t result;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		goto done;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "u", &result))
+		goto done;
+
+	l_info("request name result = %d", result);
+
+done:
+	return;
+}
+
+static void request_name_tester_callback(struct l_dbus_message *message,
+							void *user_data)
+{
+	const char *error, *text;
+	uint32_t result;
+	struct data *data = user_data;
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("error=%s", error);
+		l_error("message=%s", text);
+		goto done;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "u", &result))
+		goto done;
+
+	l_info("request tester name result = %d", result);
+
+	l_dbus_method_call(data->dbus, "org.freedesktop.DBus",
+			"/org/freedesktop/DBus",
+			"org.freedesktop.DBus", "ReleaseName",
+			release_name_setup,
+			release_name_callback, data, NULL);
+done:
+	return;
+}
+
+static void disconnected(struct l_dbus *dbus, void *user_data)
+{
+	struct tm *test_tm = user_data;
+
+	l_main_quit();
+
+	l_info("client disconnected");
+
+	l_timeout_remove(timeout);
+	timeout = NULL;
+
+	assert(test_tm->tm_sec == 1);
+	assert(test_tm->tm_min == 2);
+	assert(test_tm->tm_hour == 3);
+	assert(test_tm->tm_year == 1970);
+	assert(test_tm->tm_isdst == 0);
+
+	assert(test_tm == tm);
+
+	failure = false;
+}
+
+static void destroy(void *user_data)
+{
+	struct tm *test_tm = user_data;
+
+	assert(test_tm == tm);
+
+	l_free(test_tm);
+}
+
+static void timeout_cb(struct l_timeout *timeout, void *user_data)
+{
+	failure = true;
+
+	l_main_quit();
+}
+
+static struct l_dbus *get_bus(void)
+{
+	struct l_dbus *dbus = NULL;
+	int i;
+
+	for (i = 0; i < 10; i++) {
+		usleep(200 * 1000);
+
+		dbus = l_dbus_new(TEST_BUS_ADDRESS);
+		if (dbus)
+			break;
+	}
+
+	return dbus;
+}
+
+static void request_tester()
+{
+	struct l_dbus *dbus = get_bus();
+	struct data data;
+
+	l_dbus_set_ready_handler(dbus, ready_tester, NULL, NULL);
+
+	data.name = SERVICE_TO_WATCH;
+	data.dbus = dbus;
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+			"/org/freedesktop/DBus",
+			"org.freedesktop.DBus", "RequestName",
+			request_name_setup,
+			request_name_tester_callback, &data, NULL);
+
+	l_main_run();
+
+	l_dbus_destroy(dbus);
+
+	exit(1);
+}
+
+int main(int argc, char *argv[])
+{
+	struct l_dbus *dbus;
+	struct l_signal *signal;
+	sigset_t mask;
+	unsigned int watch_id;
+	struct data data;
+
+	l_log_set_stderr();
+
+	if (argc > 1 && !strcmp(argv[1], "test"))
+		request_tester();
+
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGINT);
+	sigaddset(&mask, SIGTERM);
+	sigaddset(&mask, SIGCHLD);
+
+	signal = l_signal_create(&mask, signal_handler, NULL, NULL);
+
+	start_dbus_daemon();
+
+	dbus = get_bus();
+
+	l_dbus_set_ready_handler(dbus, ready_callback, argv[0], NULL);
+
+	tm = l_new(struct tm, 1);
+
+	tm->tm_sec = 1;
+	tm->tm_min = 2;
+	tm->tm_hour = 3;
+	tm->tm_year = 1970;
+	tm->tm_isdst = 0;
+
+	failure = true;
+
+	watch_id = l_dbus_add_disconnect_watch(dbus,
+					SERVICE_TO_WATCH,
+					disconnected,
+					tm,
+					destroy);
+
+	data.name = SERVICE_MONITOR;
+	data.dbus = dbus;
+
+	l_dbus_method_call(dbus, "org.freedesktop.DBus",
+			"/org/freedesktop/DBus",
+			"org.freedesktop.DBus", "RequestName",
+			request_name_setup,
+			request_name_callback, &data, NULL);
+
+	timeout = l_timeout_create(5, timeout_cb, dbus, NULL);
+
+	l_main_run();
+
+	l_dbus_remove_watch(dbus, watch_id);
+
+	if (dbus_daemon_pid > 0)
+		kill(dbus_daemon_pid, SIGKILL);
+
+	if (test_daemon_pid > 0)
+		kill(test_daemon_pid, SIGKILL);
+
+	l_dbus_destroy(dbus);
+
+	l_signal_remove(signal);
+
+	l_timeout_remove(timeout);
+
+	assert(failure == false);
+
+	l_info("dbus watch test passed");
+
+	return 0;
+}
-- 
1.8.3.1

Re: [PATCH 2/3] dbus: Add disconnect watch support

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 11296 bytes --]

Hi Jukka,

On 02/10/2015 09:46 AM, Jukka Rissanen wrote:
> Allow caller to monitor the disconnect status of a service.
> ---
>   Makefile.am      |   1 +
>   ell/dbus-watch.c | 267 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/dbus.h       |  17 ++++
>   3 files changed, 285 insertions(+)
>   create mode 100644 ell/dbus-watch.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 50f81eb..3dcc7ee 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -63,6 +63,7 @@ ell_libell_la_SOURCES = $(linux_headers) \
>   			ell/dbus-message.c \
>   			ell/dbus-util.c \
>   			ell/dbus-service.c \
> +			ell/dbus-watch.c \
>   			ell/gvariant-private.h \
>   			ell/gvariant-util.c \
>   			ell/siphash-private.h \
> diff --git a/ell/dbus-watch.c b/ell/dbus-watch.c
> new file mode 100644
> index 0000000..005bc83
> --- /dev/null
> +++ b/ell/dbus-watch.c
> @@ -0,0 +1,267 @@
> +/*
> + *
> + *  Embedded Linux library
> + *
> + *  Copyright (C) 2015  Intel Corporation. All rights reserved.
> + *
> + *  This library is free software; you can redistribute it and/or
> + *  modify it under the terms of the GNU Lesser General Public
> + *  License as published by the Free Software Foundation; either
> + *  version 2.1 of the License, or (at your option) any later version.
> + *
> + *  This library 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
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include <stdarg.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +#include "log.h"
> +#include "util.h"
> +#include "queue.h"
> +#include "string.h"
> +#include "dbus.h"
> +#include "dbus-private.h"
> +#include "private.h"
> +
> +struct filter_data {
> +	struct l_dbus *dbus;
> +	l_dbus_message_func_t handle_func;
> +	l_dbus_watch_func_t connect_func;
> +	l_dbus_watch_func_t disconnect_func;
> +	char *name;
> +	char *owner;
> +	char *path;
> +	char *interface;
> +	char *member;
> +	char *argument;
> +	void *user_data;
> +	l_dbus_destroy_func_t destroy_func;
> +};
> +
> +static void format_rule(struct filter_data *data, char *rule, size_t size)
> +{
> +	const char *sender;
> +	int offset;
> +
> +	offset = snprintf(rule, size, "type='signal'");
> +	sender = data->name ? : data->owner;
> +
> +	if (sender)
> +		offset += snprintf(rule + offset, size - offset,
> +				",sender='%s'", sender);
> +	if (data->path)
> +		offset += snprintf(rule + offset, size - offset,
> +				",path='%s'", data->path);
> +	if (data->interface)
> +		offset += snprintf(rule + offset, size - offset,
> +				",interface='%s'", data->interface);
> +	if (data->member)
> +		offset += snprintf(rule + offset, size - offset,
> +				",member='%s'", data->member);
> +	if (data->argument)
> +		snprintf(rule + offset, size - offset,
> +				",arg0='%s'", data->argument);
> +}
> +
> +static void add_match(struct filter_data *data, l_dbus_message_func_t filter)
> +{
> +	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
> +
> +	format_rule(data, rule, sizeof(rule));
> +
> +	l_dbus_bus_add_match(data->dbus, rule);
> +
> +	data->handle_func = filter;
> +}
> +
> +static void remove_match(struct filter_data *data)
> +{
> +	char rule[DBUS_MAXIMUM_MATCH_RULE_LENGTH];
> +
> +	format_rule(data, rule, sizeof(rule));
> +
> +	l_dbus_bus_remove_match(data->dbus, rule);
> +}
> +
> +static struct filter_data *filter_data_get(struct l_dbus *dbus,
> +					l_dbus_message_func_t filter,
> +					const char *sender,
> +					const char *path,
> +					const char *interface,
> +					const char *member,
> +					const char *argument,
> +					l_dbus_watch_func_t connect_func,
> +					l_dbus_watch_func_t disconnect_func,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy)
> +{
> +	struct filter_data *data;
> +	const char *name = NULL, *owner = NULL;
> +
> +	if (sender) {
> +		if (sender[0] == ':')
> +			owner = sender;
> +		else
> +			name = sender;
> +	}
> +
> +	data = l_new(struct filter_data, 1);
> +
> +	data->dbus = dbus;
> +	data->handle_func = filter;
> +	data->connect_func = connect_func;
> +	data->disconnect_func = disconnect_func;
> +	data->name = l_strdup(name);
> +	data->owner = l_strdup(owner);
> +	data->path = l_strdup(path);
> +	data->interface = l_strdup(interface);
> +	data->member = l_strdup(member);
> +	data->argument = l_strdup(argument);
> +	data->user_data = user_data;
> +	data->destroy_func = destroy;
> +
> +	add_match(data, filter);
> +
> +	return data;
> +}
> +
> +static void filter_data_destroy(void *user_data)
> +{
> +	struct filter_data *data = user_data;
> +
> +	remove_match(data);
> +
> +	l_free(data->name);
> +	l_free(data->owner);
> +	l_free(data->path);
> +	l_free(data->interface);
> +	l_free(data->member);
> +	l_free(data->argument);
> +
> +	if (data->destroy_func)
> +		data->destroy_func(data->user_data);
> +
> +	l_free(data);
> +}
> +
> +static void message_filter(struct l_dbus_message *message, void *user_data)
> +{
> +	struct filter_data *data = user_data;
> +	const char *sender, *path, *iface, *member, *arg = NULL;
> +
> +	if (_dbus_message_get_type(message) != DBUS_MESSAGE_TYPE_SIGNAL)
> +		return;
> +
> +	sender = l_dbus_message_get_sender(message);
> +	if (!sender)
> +		return;
> +
> +	path = l_dbus_message_get_path(message);
> +	iface = l_dbus_message_get_interface(message);
> +	member = l_dbus_message_get_member(message);
> +
> +	l_dbus_message_get_arguments(message, "s", &arg);
> +
> +	if (data->owner && strcmp(sender, data->owner))
> +		goto out;
> +
> +	if (data->path && strcmp(path, data->path))
> +		goto out;
> +
> +	if (data->interface && strcmp(iface, data->interface))
> +		goto out;
> +
> +	if (data->member && strcmp(member, data->member))
> +		goto out;
> +
> +	if (arg && data->argument && strcmp(arg, data->argument))
> +		goto out;
> +
> +	if (data->handle_func)
> +		data->handle_func(message, data);
> +
> +out:
> +	return;
> +}
> +
> +static void service_filter(struct l_dbus_message *message, void *user_data)
> +{
> +	struct filter_data *data = user_data;
> +	char *name, *old, *new;
> +
> +	if (!l_dbus_message_get_arguments(message, "sss",
> +						&name, &old, &new)) {
> +		l_error("Invalid arguments for NameOwnerChanged signal");
> +		return;
> +	}
> +
> +	if (*new == '\0') {
> +		if (data->disconnect_func)
> +			data->disconnect_func(data->dbus, data->user_data);
> +	} else {
> +		if (data->connect_func)
> +			data->connect_func(data->dbus, data->user_data);
> +	}
> +}
> +
> +LIB_EXPORT unsigned int l_dbus_add_service_watch(struct l_dbus *dbus,
> +					const char *name,
> +					l_dbus_watch_func_t connect_func,
> +					l_dbus_watch_func_t disconnect_func,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy)
> +{
> +	struct filter_data *data;
> +
> +	if (!name)
> +		return 0;
> +
> +	if (connect_func) {
> +		l_error("Connect watch not yet supported");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	data = filter_data_get(dbus, service_filter,
> +				DBUS_SERVICE_DBUS, DBUS_PATH_DBUS,
> +				DBUS_INTERFACE_DBUS, "NameOwnerChanged",
> +				name,
> +				connect_func,
> +				disconnect_func,
> +				user_data,
> +				destroy);
> +	if (!data)
> +		return 0;
> +
> +	return l_dbus_register(dbus, message_filter, data,
> +							filter_data_destroy);

So this is the reason why you have re-entrancy problems.  You are trying 
to use l_dbus_register and l_dbus_unregister.  So when the signal_list 
hash is traversed, the agent tries to call l_dbus_unregister and things die.

I suggest that you rework this part to maintain its own data structure 
instead of using l_dbus_register, or make that part smarter.  You can 
handle re-entrancy like gdbus does now.  Since you already used most of 
the gdbus design, might as well take the rest of it (e.g. maintain a 
list of items to be removed and a re-entrancy guard).

Another idea might be to use the return value from the callback in order 
to remove the watch right then and there.  Similar to how g_io_add_watch 
and its callback works.

However, as I mentioned before, we need a way smarter data structure 
here long term.  A multi-level tree or something else.  The current 
l_hashmap_foreach traversal of signal_list needs to go away.

For the purposes of getting something working in the near future, I'm 
fine with this approach.

> +}
> +
> +LIB_EXPORT unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
> +					const char *name,
> +					l_dbus_watch_func_t disconnect_func,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy)
> +{
> +	return l_dbus_add_service_watch(dbus, name, NULL, disconnect_func,
> +							user_data, destroy);
> +}
> +
> +LIB_EXPORT bool l_dbus_remove_watch(struct l_dbus *dbus, unsigned int id)
> +{
> +	return l_dbus_unregister(dbus, id);
> +}

These three functions should probably be in dbus.c and the needed 
utilities exposed via dbus-private.h.

> diff --git a/ell/dbus.h b/ell/dbus.h
> index 9fcce03..8952dcf 100644
> --- a/ell/dbus.h
> +++ b/ell/dbus.h
> @@ -36,6 +36,8 @@ extern "C" {
>   #define DBUS_PATH_DBUS		"/org/freedesktop/DBus"
>   #define DBUS_INTERFACE_DBUS	"org.freedesktop.DBus"
>
> +#define DBUS_MAXIMUM_MATCH_RULE_LENGTH	1024
> +
>   enum l_dbus_bus {
>   	L_DBUS_SYSTEM_BUS,
>   	L_DBUS_SESSION_BUS,
> @@ -52,6 +54,8 @@ typedef void (*l_dbus_debug_func_t) (const char *str, void *user_data);
>   typedef void (*l_dbus_destroy_func_t) (void *user_data);
>   typedef void (*l_dbus_interface_setup_func_t) (struct l_dbus_interface *);
>
> +typedef void (*l_dbus_watch_func_t) (struct l_dbus *dbus, void *user_data);
> +
>   struct l_dbus *l_dbus_new(const char *address);
>   struct l_dbus *l_dbus_new_default(enum l_dbus_bus bus);
>   void l_dbus_destroy(struct l_dbus *dbus);
> @@ -204,6 +208,19 @@ bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
>   void l_dbus_bus_add_match(struct l_dbus *dbus, const char *rule);
>   void l_dbus_bus_remove_match(struct l_dbus *dbus, const char *rule);
>
> +unsigned int l_dbus_add_service_watch(struct l_dbus *dbus,
> +					const char *name,
> +					l_dbus_watch_func_t connect_func,
> +					l_dbus_watch_func_t disconnect_func,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy);
> +unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
> +					const char *name,
> +					l_dbus_watch_func_t disconnect_func,
> +					void *user_data,
> +					l_dbus_destroy_func_t destroy);
> +bool l_dbus_remove_watch(struct l_dbus *dbus, unsigned int id);
> +
>   #ifdef __cplusplus
>   }
>   #endif
>

Regards,
-Denis