* [PATCH v2] gdbus: Fix calling GetAll while GetManagedObject is pending
@ 2017-08-14 10:46 Luiz Augusto von Dentz
2017-08-14 17:16 ` Vinicius Costa Gomes
0 siblings, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2017-08-14 10:46 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If proxies are created while the client is not ready put them into a
pending list so only if they are not found in GetManagedObject reply
call GetAll.
---
gdbus/client.c | 96 ++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 59 insertions(+), 37 deletions(-)
diff --git a/gdbus/client.c b/gdbus/client.c
index a011e19c3..8cabc5590 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -76,6 +76,8 @@ struct GDBusProxy {
void *prop_data;
GDBusProxyFunction removed_func;
void *removed_data;
+ DBusPendingCall *get_all_call;
+ gboolean pending;
};
struct prop_entry {
@@ -278,6 +280,17 @@ static void update_properties(GDBusProxy *proxy, DBusMessageIter *iter,
}
}
+static void proxy_added(GDBusClient *client, GDBusProxy *proxy)
+{
+ if (!proxy->pending)
+ return;
+
+ if (client->proxy_added)
+ client->proxy_added(proxy, client->user_data);
+
+ proxy->pending = FALSE;
+}
+
static void get_all_properties_reply(DBusPendingCall *call, void *user_data)
{
GDBusProxy *proxy = user_data;
@@ -286,6 +299,8 @@ static void get_all_properties_reply(DBusPendingCall *call, void *user_data)
DBusMessageIter iter;
DBusError error;
+ g_dbus_client_ref(client);
+
dbus_error_init(&error);
if (dbus_set_error_from_message(&error, reply) == TRUE) {
@@ -298,15 +313,13 @@ static void get_all_properties_reply(DBusPendingCall *call, void *user_data)
update_properties(proxy, &iter, FALSE);
done:
- if (g_list_find(client->proxy_list, proxy) == NULL) {
- if (client->proxy_added)
- client->proxy_added(proxy, client->user_data);
-
- client->proxy_list = g_list_append(client->proxy_list, proxy);
- }
+ proxy_added(client, proxy);
dbus_message_unref(reply);
+ dbus_pending_call_unref(proxy->get_all_call);
+ proxy->get_all_call = NULL;
+
g_dbus_client_unref(client);
}
@@ -315,7 +328,9 @@ static void get_all_properties(GDBusProxy *proxy)
GDBusClient *client = proxy->client;
const char *service_name = client->service_name;
DBusMessage *msg;
- DBusPendingCall *call;
+
+ if (proxy->get_all_call)
+ return;
msg = dbus_message_new_method_call(service_name, proxy->obj_path,
DBUS_INTERFACE_PROPERTIES, "GetAll");
@@ -326,28 +341,24 @@ static void get_all_properties(GDBusProxy *proxy)
DBUS_TYPE_INVALID);
if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
- &call, -1) == FALSE) {
+ &proxy->get_all_call, -1) == FALSE) {
dbus_message_unref(msg);
return;
}
- g_dbus_client_ref(client);
-
- dbus_pending_call_set_notify(call, get_all_properties_reply,
- proxy, NULL);
- dbus_pending_call_unref(call);
+ dbus_pending_call_set_notify(proxy->get_all_call,
+ get_all_properties_reply, proxy, NULL);
dbus_message_unref(msg);
}
-static GDBusProxy *proxy_lookup(GDBusClient *client, const char *path,
+static GDBusProxy *proxy_lookup(GList *list, const char *path,
const char *interface)
{
- GList *list;
+ GList *l;
- for (list = g_list_first(client->proxy_list); list;
- list = g_list_next(list)) {
- GDBusProxy *proxy = list->data;
+ for (l = g_list_first(list); l; l = g_list_next(l)) {
+ GDBusProxy *proxy = l->data;
if (g_str_equal(proxy->interface, interface) == TRUE &&
g_str_equal(proxy->obj_path, path) == TRUE)
@@ -424,6 +435,7 @@ static GDBusProxy *proxy_new(GDBusClient *client, const char *path,
proxy->interface,
properties_changed,
proxy, NULL);
+ proxy->pending = TRUE;
return g_dbus_proxy_ref(proxy);
}
@@ -478,15 +490,19 @@ GDBusProxy *g_dbus_proxy_new(GDBusClient *client, const char *path,
if (client == NULL)
return NULL;
- proxy = proxy_lookup(client, path, interface);
+ proxy = proxy_lookup(client->proxy_list, path, interface);
if (proxy)
return g_dbus_proxy_ref(proxy);
+
proxy = proxy_new(client, path, interface);
if (proxy == NULL)
return NULL;
- get_all_properties(proxy);
+ client->proxy_list = g_list_append(client->proxy_list, proxy);
+
+ if (client->connected && !client->get_objects_call)
+ get_all_properties(proxy);
return g_dbus_proxy_ref(proxy);
}
@@ -509,6 +525,11 @@ void g_dbus_proxy_unref(GDBusProxy *proxy)
if (__sync_sub_and_fetch(&proxy->ref_count, 1) > 0)
return;
+ if (proxy->get_all_call != NULL) {
+ dbus_pending_call_cancel(proxy->get_all_call);
+ dbus_pending_call_unref(proxy->get_all_call);
+ }
+
g_hash_table_destroy(proxy->prop_list);
g_free(proxy->obj_path);
@@ -916,15 +937,15 @@ gboolean g_dbus_proxy_set_removed_watch(GDBusProxy *proxy,
return TRUE;
}
-static void refresh_properties(GDBusClient *client)
+static void refresh_properties(GList *list)
{
- GList *list;
+ GList *l;
- for (list = g_list_first(client->proxy_list); list;
- list = g_list_next(list)) {
+ for (l = g_list_first(list); l; l = g_list_next(l)) {
GDBusProxy *proxy = list->data;
- get_all_properties(proxy);
+ if (proxy->pending)
+ get_all_properties(proxy);
}
}
@@ -939,22 +960,21 @@ static void parse_properties(GDBusClient *client, const char *path,
if (g_str_equal(interface, DBUS_INTERFACE_PROPERTIES) == TRUE)
return;
- proxy = proxy_lookup(client, path, interface);
- if (proxy) {
+ proxy = proxy_lookup(client->proxy_list, path, interface);
+ if (proxy && !proxy->pending) {
update_properties(proxy, iter, FALSE);
return;
}
- proxy = proxy_new(client, path, interface);
- if (proxy == NULL)
- return;
+ if (!proxy) {
+ proxy = proxy_new(client, path, interface);
+ if (proxy == NULL)
+ return;
+ }
update_properties(proxy, iter, FALSE);
- if (client->proxy_added)
- client->proxy_added(proxy, client->user_data);
-
- client->proxy_list = g_list_append(client->proxy_list, proxy);
+ proxy_added(client, proxy);
}
static void parse_interfaces(GDBusClient *client, const char *path,
@@ -1103,6 +1123,8 @@ done:
dbus_pending_call_unref(client->get_objects_call);
client->get_objects_call = NULL;
+ refresh_properties(client->proxy_list);
+
g_dbus_client_unref(client);
}
@@ -1115,7 +1137,7 @@ static void get_managed_objects(GDBusClient *client)
if ((!client->proxy_added && !client->proxy_removed) ||
!client->root_path) {
- refresh_properties(client);
+ refresh_properties(client->proxy_list);
return;
}
@@ -1152,11 +1174,11 @@ static void service_connect(DBusConnection *conn, void *user_data)
client->connected = TRUE;
+ get_managed_objects(client);
+
if (client->connect_func)
client->connect_func(conn, client->connect_data);
- get_managed_objects(client);
-
g_dbus_client_unref(client);
}
--
2.13.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] gdbus: Fix calling GetAll while GetManagedObject is pending
2017-08-14 10:46 [PATCH v2] gdbus: Fix calling GetAll while GetManagedObject is pending Luiz Augusto von Dentz
@ 2017-08-14 17:16 ` Vinicius Costa Gomes
2017-08-15 8:22 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 5+ messages in thread
From: Vinicius Costa Gomes @ 2017-08-14 17:16 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth
Hi Luiz,
Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> If proxies are created while the client is not ready put them into a
> pending list so only if they are not found in GetManagedObject reply
> call GetAll.
> ---
> gdbus/client.c | 96 ++++++++++++++++++++++++++++++++++++----------------------
> 1 file changed, 59 insertions(+), 37 deletions(-)
>
[...]
> @@ -424,6 +435,7 @@ static GDBusProxy *proxy_new(GDBusClient *client, const char *path,
> proxy->interface,
> properties_changed,
> proxy, NULL);
> + proxy->pending = TRUE;
>
> return g_dbus_proxy_ref(proxy);
> }
> @@ -478,15 +490,19 @@ GDBusProxy *g_dbus_proxy_new(GDBusClient *client, const char *path,
> if (client == NULL)
> return NULL;
>
> - proxy = proxy_lookup(client, path, interface);
> + proxy = proxy_lookup(client->proxy_list, path, interface);
> if (proxy)
> return g_dbus_proxy_ref(proxy);
>
> +
Extra empty line here.
> proxy = proxy_new(client, path, interface);
> if (proxy == NULL)
> return NULL;
>
Just that nitpick, looking good.
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] gdbus: Fix calling GetAll while GetManagedObject is pending
2017-08-14 17:16 ` Vinicius Costa Gomes
@ 2017-08-15 8:22 ` Luiz Augusto von Dentz
2017-08-15 14:08 ` Kai Ruhnau
0 siblings, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2017-08-15 8:22 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth@vger.kernel.org
Hi,
On Mon, Aug 14, 2017 at 8:16 PM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Hi Luiz,
>
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> If proxies are created while the client is not ready put them into a
>> pending list so only if they are not found in GetManagedObject reply
>> call GetAll.
>> ---
>> gdbus/client.c | 96 ++++++++++++++++++++++++++++++++++++----------------------
>> 1 file changed, 59 insertions(+), 37 deletions(-)
>>
>
> [...]
>
>> @@ -424,6 +435,7 @@ static GDBusProxy *proxy_new(GDBusClient *client, const char *path,
>> proxy->interface,
>> properties_changed,
>> proxy, NULL);
>> + proxy->pending = TRUE;
>>
>> return g_dbus_proxy_ref(proxy);
>> }
>> @@ -478,15 +490,19 @@ GDBusProxy *g_dbus_proxy_new(GDBusClient *client, const char *path,
>> if (client == NULL)
>> return NULL;
>>
>> - proxy = proxy_lookup(client, path, interface);
>> + proxy = proxy_lookup(client->proxy_list, path, interface);
>> if (proxy)
>> return g_dbus_proxy_ref(proxy);
>>
>> +
>
> Extra empty line here.
Fixed this one, thanks for pointing out.
>> proxy = proxy_new(client, path, interface);
>> if (proxy == NULL)
>> return NULL;
>>
>
> Just that nitpick, looking good.
Applied.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] gdbus: Fix calling GetAll while GetManagedObject is pending
2017-08-15 8:22 ` Luiz Augusto von Dentz
@ 2017-08-15 14:08 ` Kai Ruhnau
0 siblings, 0 replies; 5+ messages in thread
From: Kai Ruhnau @ 2017-08-15 14:08 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Vinicius Costa Gomes
Cc: linux-bluetooth@vger.kernel.org
Hi,
On 15.08.2017 10:22, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Mon, Aug 14, 2017 at 8:16 PM, Vinicius Costa Gomes
> <vinicius.gomes@intel.com> wrote:
>> Hi Luiz,
>>
>> Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
>>
>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>
>>> If proxies are created while the client is not ready put them into a
>>> pending list so only if they are not found in GetManagedObject reply
>>> call GetAll.
>>> ---
>>> gdbus/client.c | 96 ++++++++++++++++++++++++++++++++++++-------------=
---------
>>> 1 file changed, 59 insertions(+), 37 deletions(-)
>>>
>> [...]
>>
>>
>>> proxy =3D proxy_new(client, path, interface);
>>> if (proxy =3D=3D NULL)
>>> return NULL;
>>>
>> Just that nitpick, looking good.
> Applied.
>
Yay! I've applied the patch against 5.43 and don't get the SIGSEGV anymore.
Thanks and cheers,
Kai
--
Kai Ruhnau
Software Manager
T:+49 202 769302 19
Target Systemelektronik GmbH & Co. KG
Heinz-Fangman-Stra=C3=9Fe 4
42287 Wuppertal
Amtsgericht Wuppertal HRA 23898
Pers=C3=B6nlich haftende Gesellschafterin
Target Systemelektronik Beteiligungs GmbH
Heinz-Fangman-Stra=C3=9Fe 4, 42287 Wuppertal
Amtsgericht Wuppertal HRB 25346
Gesch=C3=A4ftsf=C3=BChrer: J=C3=BCrgen Stein
--
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] gdbus: Fix calling GetAll while GetManagedObject is pending
@ 2017-08-14 9:43 Luiz Augusto von Dentz
0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2017-08-14 9:43 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
If proxies are created while the client is not ready put them into a
pending list so only if they are not found in GetManagedObject reply
call GetAll.
---
gdbus/client.c | 96 ++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 59 insertions(+), 37 deletions(-)
diff --git a/gdbus/client.c b/gdbus/client.c
index a011e19c3..8cabc5590 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -76,6 +76,8 @@ struct GDBusProxy {
void *prop_data;
GDBusProxyFunction removed_func;
void *removed_data;
+ DBusPendingCall *get_all_call;
+ gboolean pending;
};
struct prop_entry {
@@ -278,6 +280,17 @@ static void update_properties(GDBusProxy *proxy, DBusMessageIter *iter,
}
}
+static void proxy_added(GDBusClient *client, GDBusProxy *proxy)
+{
+ if (!proxy->pending)
+ return;
+
+ if (client->proxy_added)
+ client->proxy_added(proxy, client->user_data);
+
+ proxy->pending = FALSE;
+}
+
static void get_all_properties_reply(DBusPendingCall *call, void *user_data)
{
GDBusProxy *proxy = user_data;
@@ -286,6 +299,8 @@ static void get_all_properties_reply(DBusPendingCall *call, void *user_data)
DBusMessageIter iter;
DBusError error;
+ g_dbus_client_ref(client);
+
dbus_error_init(&error);
if (dbus_set_error_from_message(&error, reply) == TRUE) {
@@ -298,15 +313,13 @@ static void get_all_properties_reply(DBusPendingCall *call, void *user_data)
update_properties(proxy, &iter, FALSE);
done:
- if (g_list_find(client->proxy_list, proxy) == NULL) {
- if (client->proxy_added)
- client->proxy_added(proxy, client->user_data);
-
- client->proxy_list = g_list_append(client->proxy_list, proxy);
- }
+ proxy_added(client, proxy);
dbus_message_unref(reply);
+ dbus_pending_call_unref(proxy->get_all_call);
+ proxy->get_all_call = NULL;
+
g_dbus_client_unref(client);
}
@@ -315,7 +328,9 @@ static void get_all_properties(GDBusProxy *proxy)
GDBusClient *client = proxy->client;
const char *service_name = client->service_name;
DBusMessage *msg;
- DBusPendingCall *call;
+
+ if (proxy->get_all_call)
+ return;
msg = dbus_message_new_method_call(service_name, proxy->obj_path,
DBUS_INTERFACE_PROPERTIES, "GetAll");
@@ -326,28 +341,24 @@ static void get_all_properties(GDBusProxy *proxy)
DBUS_TYPE_INVALID);
if (g_dbus_send_message_with_reply(client->dbus_conn, msg,
- &call, -1) == FALSE) {
+ &proxy->get_all_call, -1) == FALSE) {
dbus_message_unref(msg);
return;
}
- g_dbus_client_ref(client);
-
- dbus_pending_call_set_notify(call, get_all_properties_reply,
- proxy, NULL);
- dbus_pending_call_unref(call);
+ dbus_pending_call_set_notify(proxy->get_all_call,
+ get_all_properties_reply, proxy, NULL);
dbus_message_unref(msg);
}
-static GDBusProxy *proxy_lookup(GDBusClient *client, const char *path,
+static GDBusProxy *proxy_lookup(GList *list, const char *path,
const char *interface)
{
- GList *list;
+ GList *l;
- for (list = g_list_first(client->proxy_list); list;
- list = g_list_next(list)) {
- GDBusProxy *proxy = list->data;
+ for (l = g_list_first(list); l; l = g_list_next(l)) {
+ GDBusProxy *proxy = l->data;
if (g_str_equal(proxy->interface, interface) == TRUE &&
g_str_equal(proxy->obj_path, path) == TRUE)
@@ -424,6 +435,7 @@ static GDBusProxy *proxy_new(GDBusClient *client, const char *path,
proxy->interface,
properties_changed,
proxy, NULL);
+ proxy->pending = TRUE;
return g_dbus_proxy_ref(proxy);
}
@@ -478,15 +490,19 @@ GDBusProxy *g_dbus_proxy_new(GDBusClient *client, const char *path,
if (client == NULL)
return NULL;
- proxy = proxy_lookup(client, path, interface);
+ proxy = proxy_lookup(client->proxy_list, path, interface);
if (proxy)
return g_dbus_proxy_ref(proxy);
+
proxy = proxy_new(client, path, interface);
if (proxy == NULL)
return NULL;
- get_all_properties(proxy);
+ client->proxy_list = g_list_append(client->proxy_list, proxy);
+
+ if (client->connected && !client->get_objects_call)
+ get_all_properties(proxy);
return g_dbus_proxy_ref(proxy);
}
@@ -509,6 +525,11 @@ void g_dbus_proxy_unref(GDBusProxy *proxy)
if (__sync_sub_and_fetch(&proxy->ref_count, 1) > 0)
return;
+ if (proxy->get_all_call != NULL) {
+ dbus_pending_call_cancel(proxy->get_all_call);
+ dbus_pending_call_unref(proxy->get_all_call);
+ }
+
g_hash_table_destroy(proxy->prop_list);
g_free(proxy->obj_path);
@@ -916,15 +937,15 @@ gboolean g_dbus_proxy_set_removed_watch(GDBusProxy *proxy,
return TRUE;
}
-static void refresh_properties(GDBusClient *client)
+static void refresh_properties(GList *list)
{
- GList *list;
+ GList *l;
- for (list = g_list_first(client->proxy_list); list;
- list = g_list_next(list)) {
+ for (l = g_list_first(list); l; l = g_list_next(l)) {
GDBusProxy *proxy = list->data;
- get_all_properties(proxy);
+ if (proxy->pending)
+ get_all_properties(proxy);
}
}
@@ -939,22 +960,21 @@ static void parse_properties(GDBusClient *client, const char *path,
if (g_str_equal(interface, DBUS_INTERFACE_PROPERTIES) == TRUE)
return;
- proxy = proxy_lookup(client, path, interface);
- if (proxy) {
+ proxy = proxy_lookup(client->proxy_list, path, interface);
+ if (proxy && !proxy->pending) {
update_properties(proxy, iter, FALSE);
return;
}
- proxy = proxy_new(client, path, interface);
- if (proxy == NULL)
- return;
+ if (!proxy) {
+ proxy = proxy_new(client, path, interface);
+ if (proxy == NULL)
+ return;
+ }
update_properties(proxy, iter, FALSE);
- if (client->proxy_added)
- client->proxy_added(proxy, client->user_data);
-
- client->proxy_list = g_list_append(client->proxy_list, proxy);
+ proxy_added(client, proxy);
}
static void parse_interfaces(GDBusClient *client, const char *path,
@@ -1103,6 +1123,8 @@ done:
dbus_pending_call_unref(client->get_objects_call);
client->get_objects_call = NULL;
+ refresh_properties(client->proxy_list);
+
g_dbus_client_unref(client);
}
@@ -1115,7 +1137,7 @@ static void get_managed_objects(GDBusClient *client)
if ((!client->proxy_added && !client->proxy_removed) ||
!client->root_path) {
- refresh_properties(client);
+ refresh_properties(client->proxy_list);
return;
}
@@ -1152,11 +1174,11 @@ static void service_connect(DBusConnection *conn, void *user_data)
client->connected = TRUE;
+ get_managed_objects(client);
+
if (client->connect_func)
client->connect_func(conn, client->connect_data);
- get_managed_objects(client);
-
g_dbus_client_unref(client);
}
--
2.13.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-15 14:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-14 10:46 [PATCH v2] gdbus: Fix calling GetAll while GetManagedObject is pending Luiz Augusto von Dentz
2017-08-14 17:16 ` Vinicius Costa Gomes
2017-08-15 8:22 ` Luiz Augusto von Dentz
2017-08-15 14:08 ` Kai Ruhnau
-- strict thread matches above, loose matches on Subject: below --
2017-08-14 9:43 Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.