Hi Manuel,
On Fri, Jul 23, 2010 at 11:37 PM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi,
>
> On Wed, Jul 21, 2010 at 6:11 PM, Manuel Naranjo <manuel@xxxxxxxxxxxx> wrote:
>> I think this is the one that really fix the problem. I see connect_watch
>> getting called and then getting into the crash. I have a nice log with the
>> tracing feature I sent the other day, here's the end of it (the hole thing
>> is almost 40 megs if someone wants just ask for it).
>
> I just figure out that our connect_watch in glib_helper.c is not quite
> right, it should be something similar as we have btio.c, specially
> this one is particular important:
>
> /* If the user aborted this connect attempt */
> if ((cond & G_IO_NVAL) || check_nval(io))
> return FALSE;
>
> It is probably because of not having this check that the cb is still
> called after bt_cancel_discovery. Of course this doesn't invalidate
> your fix to bt_cancel_discovery itself, but I guess this should also
> be included to safe that the callback is not called after cancelling
> the discovery.
Can you try the patch attached to this email?
--
Luiz Augusto von Dentz
Computer Engineer
From 28953f5b419693d8281f057055a34d9bc3e4442e Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.dentz-von@xxxxxxxxx>
Date: Wed, 28 Jul 2010 16:41:12 +0300
Subject: [PATCH] core: fix not handling sdp connection errors properly
---
src/glib-helper.c | 145 +++++++++++++++++++++++++++++++++++++----------------
1 files changed, 101 insertions(+), 44 deletions(-)
diff --git a/src/glib-helper.c b/src/glib-helper.c
index 41f5e3c..afd664e 100644
--- a/src/glib-helper.c
+++ b/src/glib-helper.c
@@ -44,6 +44,8 @@
#include <glib.h>
#include "glib-helper.h"
+#include "btio.h"
+#include "log.h"
/* Number of seconds to keep a sdp_session_t in the cache */
#define CACHE_TIMEOUT 2
@@ -98,7 +100,7 @@ static sdp_session_t *get_sdp_session(const bdaddr_t *src, const bdaddr_t *dst)
return session;
}
- return sdp_connect(src, dst, SDP_NON_BLOCKING);
+ return NULL;
}
static void cache_sdp_session(bdaddr_t *src, bdaddr_t *dst,
@@ -143,6 +145,7 @@ struct search_context {
bdaddr_t src;
bdaddr_t dst;
sdp_session_t *session;
+ GIOChannel *io;
bt_callback_t cb;
bt_destroy_t destroy;
gpointer user_data;
@@ -245,56 +248,99 @@ failed:
return FALSE;
}
-static gboolean connect_watch(GIOChannel *chan, GIOCondition cond, gpointer user_data)
+static int sdp_search(struct search_context *ctxt)
{
- struct search_context *ctxt = user_data;
+ GIOChannel *chan;
sdp_list_t *search, *attrids;
uint32_t range = 0x0000ffff;
- socklen_t len;
- int sk, err = 0;
-
- sk = g_io_channel_unix_get_fd(chan);
- ctxt->io_id = 0;
+ int err, sk;
- len = sizeof(err);
- if (getsockopt(sk, SOL_SOCKET, SO_ERROR, &err, &len) < 0) {
- err = errno;
- goto failed;
- }
-
- if (err != 0)
- goto failed;
-
- if (sdp_set_notify(ctxt->session, search_completed_cb, ctxt) < 0) {
- err = EIO;
- goto failed;
- }
+ err = sdp_set_notify(ctxt->session, search_completed_cb, ctxt);
+ if (err < 0)
+ return err;
search = sdp_list_append(NULL, &ctxt->uuid);
attrids = sdp_list_append(NULL, &range);
- if (sdp_service_search_attr_async(ctxt->session,
- search, SDP_ATTR_REQ_RANGE, attrids) < 0) {
- sdp_list_free(attrids, NULL);
- sdp_list_free(search, NULL);
- err = EIO;
- goto failed;
- }
+ err = sdp_service_search_attr_async(ctxt->session, search,
+ SDP_ATTR_REQ_RANGE, attrids);
sdp_list_free(attrids, NULL);
sdp_list_free(search, NULL);
+ if (err < 0)
+ return err;
+
+ sk = sdp_get_socket(ctxt->session);
+ chan = g_io_channel_unix_new(sk);
/* Set callback responsible for update the internal SDP transaction */
ctxt->io_id = g_io_add_watch(chan,
G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
search_process_cb, ctxt);
- return FALSE;
+
+ return 0;
+}
+
+static void connect_watch(GIOChannel *chan, GError *gerr, gpointer user_data)
+{
+ struct search_context *ctxt = user_data;
+ int sk, err = 0;
+
+ if (gerr) {
+ err = -EHOSTDOWN;
+ goto failed;
+ }
+
+ g_io_channel_set_close_on_unref(ctxt->io, FALSE);
+ g_io_channel_unref(ctxt->io);
+ ctxt->io = NULL;
+
+ sk = g_io_channel_unix_get_fd(chan);
+ ctxt->session = sdp_create(sk, SDP_NON_BLOCKING);
+ if (ctxt->session == NULL) {
+ err = -ENOMEM;
+ goto failed;
+ }
+
+ err = sdp_search(ctxt);
+ if (err < 0)
+ goto failed;
+
+ return;
failed:
+ if (ctxt->session) {
+ sdp_close(ctxt->session);
+ ctxt->session = NULL;
+ }
+
+ if (ctxt->io) {
+ g_io_channel_shutdown(ctxt->io, TRUE, NULL);
+ g_io_channel_unref(ctxt->io);
+ ctxt->io = NULL;
+ }
+
+ if (ctxt->cb)
+ ctxt->cb(NULL, err, ctxt->user_data);
+
+ search_context_cleanup(ctxt);
+}
+
+static gboolean sdp_resume(gpointer data)
+{
+ struct search_context *ctxt = data;
+ int err;
+
+ ctxt->io_id = 0;
+
+ err = sdp_search(ctxt);
+ if (err == 0)
+ return FALSE;
+
sdp_close(ctxt->session);
ctxt->session = NULL;
if (ctxt->cb)
- ctxt->cb(NULL, -err, ctxt->user_data);
+ ctxt->cb(NULL, err, ctxt->user_data);
search_context_cleanup(ctxt);
@@ -306,15 +352,11 @@ static int create_search_context(struct search_context **ctxt,
uuid_t *uuid)
{
sdp_session_t *s;
- GIOChannel *chan;
+ GError *err = NULL;
if (!ctxt)
return -EINVAL;
- s = get_sdp_session(src, dst);
- if (!s)
- return -errno;
-
*ctxt = g_try_malloc0(sizeof(struct search_context));
if (!*ctxt) {
sdp_close(s);
@@ -323,14 +365,27 @@ static int create_search_context(struct search_context **ctxt,
bacpy(&(*ctxt)->src, src);
bacpy(&(*ctxt)->dst, dst);
- (*ctxt)->session = s;
(*ctxt)->uuid = *uuid;
- chan = g_io_channel_unix_new(sdp_get_socket(s));
- (*ctxt)->io_id = g_io_add_watch(chan,
- G_IO_OUT | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
- connect_watch, *ctxt);
- g_io_channel_unref(chan);
+ s = get_sdp_session(src, dst);
+ if (s != NULL) {
+ (*ctxt)->session = s;
+ (*ctxt)->io_id = g_idle_add(sdp_resume, *ctxt);
+ return 0;
+ }
+
+ (*ctxt)->io = bt_io_connect(BT_IO_L2CAP, connect_watch, *ctxt,
+ NULL, &err,
+ BT_IO_OPT_SOURCE_BDADDR, src,
+ BT_IO_OPT_DEST_BDADDR, dst,
+ BT_IO_OPT_PSM, SDP_PSM,
+ BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_SDP,
+ BT_IO_OPT_INVALID);
+ if ((*ctxt)->io == NULL) {
+ g_error_free(err);
+ search_context_cleanup(*ctxt);
+ return -EINVAL;
+ }
return 0;
}
@@ -391,15 +446,17 @@ int bt_cancel_discovery(const bdaddr_t *src, const bdaddr_t *dst)
return -ENODATA;
ctxt = match->data;
- if (!ctxt->session)
- return -ENOTCONN;
-
if (ctxt->io_id)
g_source_remove(ctxt->io_id);
if (ctxt->session)
sdp_close(ctxt->session);
+ if (ctxt->io) {
+ g_io_channel_shutdown(ctxt->io, TRUE, NULL);
+ g_io_channel_unref(ctxt->io);
+ }
+
search_context_cleanup(ctxt);
return 0;
}
--
1.7.0.4
