Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 3/4] Fix calling watch callbacks after it has been removed
@ 2010-09-06 13:39 Luiz Augusto von Dentz
  0 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2010-09-06 13:39 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>

Pending call should be removed if the watch is removed since the
application no longer expect that to be reached and may already freed the
data associated with it.
---
 gdbus/watch.c |   79 +++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/gdbus/watch.c b/gdbus/watch.c
index 8ad4815..c0dcc93 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -43,11 +43,21 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 static guint listener_id = 0;
 static GSList *listeners = NULL;
 
+struct service_data {
+	DBusConnection *conn;
+	DBusPendingCall *call;
+	char *name;
+	const char *owner;
+	guint id;
+	struct filter_callback *callback;
+};
+
 struct filter_callback {
 	GDBusWatchFunction conn_func;
 	GDBusWatchFunction disc_func;
 	GDBusSignalFunction signal_func;
 	GDBusDestroyFunction destroy_func;
+	struct service_data *data;
 	void *user_data;
 	guint id;
 };
@@ -302,7 +312,7 @@ static struct filter_callback *filter_data_add_callback(
 {
 	struct filter_callback *cb = NULL;
 
-	cb = g_new(struct filter_callback, 1);
+	cb = g_new0(struct filter_callback, 1);
 
 	cb->conn_func = connect;
 	cb->disc_func = disconnect;
@@ -319,6 +329,24 @@ static struct filter_callback *filter_data_add_callback(
 	return cb;
 }
 
+static void service_data_free(struct service_data *data)
+{
+	struct filter_callback *callback = data->callback;
+
+	dbus_connection_unref(data->conn);
+
+	if (data->call)
+		dbus_pending_call_unref(data->call);
+
+	if (data->id)
+		g_source_remove(data->id);
+
+	g_free(data->name);
+	g_free(data);
+
+	callback->data = NULL;
+}
+
 static gboolean filter_data_remove_callback(struct filter_data *data,
 						struct filter_callback *cb)
 {
@@ -327,6 +355,13 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
 	data->callbacks = g_slist_remove(data->callbacks, cb);
 	data->processed = g_slist_remove(data->processed, cb);
 
+	/* Cancel pending operations */
+	if (cb->data) {
+		if (cb->data->call)
+			dbus_pending_call_cancel(cb->data->call);
+		service_data_free(cb->data);
+	}
+
 	if (cb->destroy_func)
 		cb->destroy_func(cb->user_data);
 
@@ -515,28 +550,14 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
-struct service_data {
-	DBusConnection *conn;
-	char *name;
-	const char *owner;
-	GDBusWatchFunction conn_func;
-	void *user_data;
-};
-
-static void service_data_free(struct service_data *data)
-{
-	dbus_connection_unref(data->conn);
-	g_free(data->name);
-	g_free(data);
-}
-
 static gboolean update_service(void *user_data)
 {
 	struct service_data *data = user_data;
+	struct filter_callback *cb = data->callback;
 
 	update_name_cache(data->name, data->owner);
-	if (data->conn_func)
-		data->conn_func(data->conn, data->user_data);
+	if (cb->conn_func)
+		cb->conn_func(data->conn, cb->user_data);
 
 	service_data_free(data);
 
@@ -575,11 +596,11 @@ done:
 	dbus_message_unref(reply);
 }
 
-static void check_service(DBusConnection *connection, const char *name,
-				GDBusWatchFunction connect, void *user_data)
+static void check_service(DBusConnection *connection,
+					const char *name,
+					struct filter_callback *callback)
 {
 	DBusMessage *message;
-	DBusPendingCall *call;
 	struct service_data *data;
 
 	data = g_try_malloc0(sizeof(*data));
@@ -590,12 +611,12 @@ static void check_service(DBusConnection *connection, const char *name,
 
 	data->conn = dbus_connection_ref(connection);
 	data->name = g_strdup(name);
-	data->conn_func = connect;
-	data->user_data = user_data;
+	data->callback = callback;
+	callback->data = data;
 
 	data->owner = check_name_cache(name);
 	if (data->owner != NULL) {
-		g_idle_add(update_service, data);
+		data->id = g_idle_add(update_service, data);
 		return;
 	}
 
@@ -611,21 +632,19 @@ static void check_service(DBusConnection *connection, const char *name,
 							DBUS_TYPE_INVALID);
 
 	if (dbus_connection_send_with_reply(connection, message,
-							&call, -1) == FALSE) {
+							&data->call, -1) == FALSE) {
 		error("Failed to execute method call");
 		g_free(data);
 		goto done;
 	}
 
-	if (call == NULL) {
+	if (data->call == NULL) {
 		error("D-Bus connection not available");
 		g_free(data);
 		goto done;
 	}
 
-	dbus_pending_call_set_notify(call, service_reply, data, g_free);
-
-	dbus_pending_call_unref(call);
+	dbus_pending_call_set_notify(data->call, service_reply, data, NULL);
 
 done:
 	dbus_message_unref(message);
@@ -654,7 +673,7 @@ guint g_dbus_add_service_watch(DBusConnection *connection, const char *name,
 		return 0;
 
 	if (connect)
-		check_service(connection, name, connect, user_data);
+		check_service(connection, name, cb);
 
 	return cb->id;
 }
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH 1/4] Do not automatically remove watches for service names
@ 2010-09-06 10:26 Luiz Augusto von Dentz
  2010-09-06 10:26 ` [PATCH 3/4] Fix calling watch callbacks after it has been removed Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2010-09-06 10:26 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto Von Dentz <luiz.dentz-von@nokia.com>

Services can be owned again so it is perfectly fine to keep the watch.
---
 gdbus/watch.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/gdbus/watch.c b/gdbus/watch.c
index 29f23e2..b686c85 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -386,18 +386,19 @@ static DBusHandlerResult service_filter(DBusConnection *connection,
 				cb->conn_func(connection, cb->user_data);
 		}
 
+		/* Only auto remove if it is a bus name watch */
+		if (data->argument[0] == ':' &&
+				(!cb->conn_func || !cb->disc_func)) {
+			filter_data_remove_callback(data, cb);
+			continue;
+		}
+
 		/* Check if the watch was removed/freed by the callback
 		 * function */
 		if (!g_slist_find(data->callbacks, cb))
 			continue;
 
 		data->callbacks = g_slist_remove(data->callbacks, cb);
-
-		if (!cb->conn_func || !cb->disc_func) {
-			g_free(cb);
-			continue;
-		}
-
 		data->processed = g_slist_append(data->processed, cb);
 	}
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH 3/4] Fix calling watch callbacks after it has been removed
@ 2010-03-01 16:09 Luiz Augusto von Dentz
  0 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2010-03-01 16:09 UTC (permalink / raw)
  To: linux-bluetooth, ofono

Pending call should be removed if the watch is removed since the
application no longer expect that to be reached and may already freed the
data associated with it.
---
 gdbus/watch.c |   79 +++++++++++++++++++++++++++++++++++---------------------
 1 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/gdbus/watch.c b/gdbus/watch.c
index a1acb72..c0dcc93 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -43,11 +43,21 @@ static DBusHandlerResult
message_filter(DBusConnection *connection,
 static guint listener_id = 0;
 static GSList *listeners = NULL;

+struct service_data {
+	DBusConnection *conn;
+	DBusPendingCall *call;
+	char *name;
+	const char *owner;
+	guint id;
+	struct filter_callback *callback;
+};
+
 struct filter_callback {
 	GDBusWatchFunction conn_func;
 	GDBusWatchFunction disc_func;
 	GDBusSignalFunction signal_func;
 	GDBusDestroyFunction destroy_func;
+	struct service_data *data;
 	void *user_data;
 	guint id;
 };
@@ -302,7 +312,7 @@ static struct filter_callback *filter_data_add_callback(
 {
 	struct filter_callback *cb = NULL;

-	cb = g_new(struct filter_callback, 1);
+	cb = g_new0(struct filter_callback, 1);

 	cb->conn_func = connect;
 	cb->disc_func = disconnect;
@@ -319,6 +329,24 @@ static struct filter_callback *filter_data_add_callback(
 	return cb;
 }

+static void service_data_free(struct service_data *data)
+{
+	struct filter_callback *callback = data->callback;
+
+	dbus_connection_unref(data->conn);
+
+	if (data->call)
+		dbus_pending_call_unref(data->call);
+
+	if (data->id)
+		g_source_remove(data->id);
+
+	g_free(data->name);
+	g_free(data);
+
+	callback->data = NULL;
+}
+
 static gboolean filter_data_remove_callback(struct filter_data *data,
 						struct filter_callback *cb)
 {
@@ -327,6 +355,13 @@ static gboolean
filter_data_remove_callback(struct filter_data *data,
 	data->callbacks = g_slist_remove(data->callbacks, cb);
 	data->processed = g_slist_remove(data->processed, cb);

+	/* Cancel pending operations */
+	if (cb->data) {
+		if (cb->data->call)
+			dbus_pending_call_cancel(cb->data->call);
+		service_data_free(cb->data);
+	}
+
 	if (cb->destroy_func)
 		cb->destroy_func(cb->user_data);

@@ -515,28 +550,14 @@ static DBusHandlerResult
message_filter(DBusConnection *connection,
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }

-struct service_data {
-	DBusConnection *conn;
-	char *name;
-	const char *owner;
-	GDBusWatchFunction conn_func;
-	void *user_data;
-};
-
-static void service_data_free(struct service_data *data)
-{
-	dbus_connection_unref(data->conn);
-	g_free(data->name);
-	g_free(data);
-}
-
 static gboolean update_service(void *user_data)
 {
 	struct service_data *data = user_data;
+	struct filter_callback *cb = data->callback;

 	update_name_cache(data->name, data->owner);
-	if (data->conn_func)
-		data->conn_func(data->conn, data->user_data);
+	if (cb->conn_func)
+		cb->conn_func(data->conn, cb->user_data);

 	service_data_free(data);

@@ -575,11 +596,11 @@ done:
 	dbus_message_unref(reply);
 }

-static void check_service(DBusConnection *connection, const char *name,
-				GDBusWatchFunction connect, void *user_data)
+static void check_service(DBusConnection *connection,
+					const char *name,
+					struct filter_callback *callback)
 {
 	DBusMessage *message;
-	DBusPendingCall *call;
 	struct service_data *data;

 	data = g_try_malloc0(sizeof(*data));
@@ -590,12 +611,12 @@ static void check_service(DBusConnection
*connection, const char *name,

 	data->conn = dbus_connection_ref(connection);
 	data->name = g_strdup(name);
-	data->conn_func = connect;
-	data->user_data = user_data;
+	data->callback = callback;
+	callback->data = data;

 	data->owner = check_name_cache(name);
 	if (data->owner != NULL) {
-		g_idle_add(update_service, data);
+		data->id = g_idle_add(update_service, data);
 		return;
 	}

@@ -611,21 +632,19 @@ static void check_service(DBusConnection
*connection, const char *name,
 							DBUS_TYPE_INVALID);

 	if (dbus_connection_send_with_reply(connection, message,
-							&call, -1) == FALSE) {
+							&data->call, -1) == FALSE) {
 		error("Failed to execute method call");
 		g_free(data);
 		goto done;
 	}

-	if (call == NULL) {
+	if (data->call == NULL) {
 		error("D-Bus connection not available");
 		g_free(data);
 		goto done;
 	}

-	dbus_pending_call_set_notify(call, service_reply, data, NULL);
-
-	dbus_pending_call_unref(call);
+	dbus_pending_call_set_notify(data->call, service_reply, data, NULL);

 done:
 	dbus_message_unref(message);
@@ -654,7 +673,7 @@ guint g_dbus_add_service_watch(DBusConnection
*connection, const char *name,
 		return 0;

 	if (connect)
-		check_service(connection, name, connect, user_data);
+		check_service(connection, name, cb);

 	return cb->id;
 }
-- 
1.6.3.3

-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-09-06 13:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-06 13:39 [PATCH 3/4] Fix calling watch callbacks after it has been removed Luiz Augusto von Dentz
  -- strict thread matches above, loose matches on Subject: below --
2010-09-06 10:26 [PATCH 1/4] Do not automatically remove watches for service names Luiz Augusto von Dentz
2010-09-06 10:26 ` [PATCH 3/4] Fix calling watch callbacks after it has been removed Luiz Augusto von Dentz
2010-09-06 13:39   ` Luiz Augusto von Dentz
2010-03-01 16:09 Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox