2012-10-17 11:49:32

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ] AVDTP: Do not keep a internal reference

From: Luiz Augusto von Dentz <[email protected]>

Don't initialize reference with 1, instead always start disconnect timer
when reference drops to 0, so in case nobody reclaims the session it
automatically disconnect after 1 second and frees the memory.
---
audio/avdtp.c | 220 ++++++++++++++++++++++++++--------------------------------
1 file changed, 97 insertions(+), 123 deletions(-)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index bd91cb6..6bfeb9b 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -387,8 +387,7 @@ struct avdtp_stream {
/* Structure describing an AVDTP connection between two devices */

struct avdtp {
- int ref;
- int free_lock;
+ unsigned int ref;

uint16_t version;

@@ -657,50 +656,6 @@ static gboolean stream_open_timeout(gpointer user_data)
return FALSE;
}

-static gboolean disconnect_timeout(gpointer user_data)
-{
- struct avdtp *session = user_data;
- struct audio_device *dev;
- gboolean stream_setup;
-
- session->dc_timer = 0;
- stream_setup = session->stream_setup;
- session->stream_setup = FALSE;
-
- dev = manager_get_device(&session->server->src, &session->dst, FALSE);
-
- if (dev && dev->sink && stream_setup)
- sink_setup_stream(dev->sink, session);
- else if (dev && dev->source && stream_setup)
- source_setup_stream(dev->source, session);
- else
- connection_lost(session, ETIMEDOUT);
-
- return FALSE;
-}
-
-static void remove_disconnect_timer(struct avdtp *session)
-{
- g_source_remove(session->dc_timer);
- session->dc_timer = 0;
- session->stream_setup = FALSE;
-}
-
-static void set_disconnect_timer(struct avdtp *session)
-{
- if (session->dc_timer)
- remove_disconnect_timer(session);
-
- if (session->device_disconnect) {
- session->dc_timer = g_idle_add(disconnect_timeout, session);
- return;
- }
-
- session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
- disconnect_timeout,
- session);
-}
-
void avdtp_error_init(struct avdtp_error *err, uint8_t category, int id)
{
err->category = category;
@@ -780,8 +735,9 @@ static void avdtp_set_state(struct avdtp *session,
}
}

-static void stream_free(struct avdtp_stream *stream)
+static void stream_free(void *data)
{
+ struct avdtp_stream *stream = data;
struct avdtp_remote_sep *rsep;

stream->lsep->info.inuse = 0;
@@ -1144,37 +1100,42 @@ static int avdtp_cancel_authorization(struct avdtp *session)
return err;

session->auth_id = 0;
+ avdtp_unref(session);

return 0;
}

-static void connection_lost(struct avdtp *session, int err)
+static void sep_free(gpointer data)
{
- char address[18];
+ struct avdtp_remote_sep *sep = data;

- ba2str(&session->dst, address);
- DBG("Disconnected from %s", address);
+ g_slist_free_full(sep->caps, g_free);
+ g_free(sep);
+}

- if (err != EACCES)
- avdtp_cancel_authorization(session);
+static void remove_disconnect_timer(struct avdtp *session)
+{
+ g_source_remove(session->dc_timer);
+ session->dc_timer = 0;
+ session->stream_setup = FALSE;
+}

- session->free_lock = 1;
+static void avdtp_free(void *data)
+{
+ struct avdtp *session = data;

- finalize_discovery(session, err);
+ DBG("%p", session);

- g_slist_foreach(session->streams, (GFunc) release_stream, session);
- session->streams = NULL;
+ g_slist_free_full(session->streams, stream_free);

- session->free_lock = 0;
+ if (session->state != AVDTP_SESSION_STATE_DISCONNECTED)
+ avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);

if (session->io) {
g_io_channel_shutdown(session->io, FALSE, NULL);
g_io_channel_unref(session->io);
- session->io = NULL;
}

- avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
-
if (session->io_id) {
g_source_remove(session->io_id);
session->io_id = 0;
@@ -1183,69 +1144,95 @@ static void connection_lost(struct avdtp *session, int err)
if (session->dc_timer)
remove_disconnect_timer(session);

- if (session->ref != 1)
- error("connection_lost: ref count not 1 after all callbacks");
- else
- avdtp_unref(session);
+ avdtp_cancel_authorization(session);
+
+ if (session->req)
+ pending_req_free(session->req);
+
+ g_slist_free_full(session->seps, sep_free);
+
+ g_free(session->buf);
+
+ g_free(session);
}

-static void sep_free(gpointer data)
+static gboolean disconnect_timeout(gpointer user_data)
{
- struct avdtp_remote_sep *sep = data;
+ struct avdtp *session = user_data;
+ struct audio_device *dev;
+ gboolean stream_setup;

- g_slist_free_full(sep->caps, g_free);
- g_free(sep);
+ session->dc_timer = 0;
+
+ if (session->ref == 0) {
+ struct avdtp_server *server = session->server;
+
+ server->sessions = g_slist_remove(server->sessions, session);
+ avdtp_free(session);
+ return FALSE;
+ }
+
+ stream_setup = session->stream_setup;
+ session->stream_setup = FALSE;
+
+ dev = manager_get_device(&session->server->src, &session->dst, FALSE);
+
+ if (dev && dev->sink && stream_setup)
+ sink_setup_stream(dev->sink, session);
+ else if (dev && dev->source && stream_setup)
+ source_setup_stream(dev->source, session);
+ else
+ connection_lost(session, ETIMEDOUT);
+
+ return FALSE;
}

-void avdtp_unref(struct avdtp *session)
+static void set_disconnect_timer(struct avdtp *session)
{
- struct avdtp_server *server;
+ if (session->dc_timer)
+ remove_disconnect_timer(session);

- if (!session)
+ if (session->device_disconnect) {
+ session->dc_timer = g_idle_add(disconnect_timeout, session);
return;
+ }

- session->ref--;
-
- DBG("%p: ref=%d", session, session->ref);
+ session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
+ disconnect_timeout,
+ session);
+}

- if (session->ref == 1) {
- if (session->state == AVDTP_SESSION_STATE_CONNECTING &&
- session->io) {
- avdtp_cancel_authorization(session);
- g_io_channel_shutdown(session->io, TRUE, NULL);
- g_io_channel_unref(session->io);
- session->io = NULL;
- avdtp_set_state(session,
- AVDTP_SESSION_STATE_DISCONNECTED);
- }
+static void connection_lost(struct avdtp *session, int err)
+{
+ char address[18];

- if (session->io)
- set_disconnect_timer(session);
- else if (!session->free_lock) /* Drop the local ref if we
- aren't connected */
- session->ref--;
- }
+ ba2str(&session->dst, address);
+ DBG("Disconnected from %s", address);

- if (session->ref > 0)
- return;
+ if (err != EACCES)
+ avdtp_cancel_authorization(session);

- server = session->server;
+ g_slist_foreach(session->streams, (GFunc) release_stream, session);
+ session->streams = NULL;

- DBG("%p: freeing session and removing from list", session);
+ finalize_discovery(session, err);

- if (session->dc_timer)
- remove_disconnect_timer(session);
+ avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
+}

- server->sessions = g_slist_remove(server->sessions, session);
+void avdtp_unref(struct avdtp *session)
+{
+ if (!session)
+ return;

- if (session->req)
- pending_req_free(session->req);
+ session->ref--;

- g_slist_free_full(session->seps, sep_free);
+ DBG("%p: ref=%d", session, session->ref);

- g_free(session->buf);
+ if (session->ref > 0)
+ return;

- g_free(session);
+ set_disconnect_timer(session);
}

struct avdtp *avdtp_ref(struct avdtp *session)
@@ -2231,12 +2218,6 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
goto failed;
}

- if (session->ref == 1 && !session->streams && !session->req)
- set_disconnect_timer(session);
-
- if (session->streams && session->dc_timer)
- remove_disconnect_timer(session);
-
if (session->req && session->req->collided) {
DBG("Collision detected");
goto next;
@@ -2383,7 +2364,7 @@ static struct avdtp *avdtp_get_internal(const bdaddr_t *src, const bdaddr_t *dst

session->server = server;
bacpy(&session->dst, dst);
- session->ref = 1;
+ set_disconnect_timer(session);
/* We don't use avdtp_set_state() here since this isn't a state change
* but just setting of the initial state */
session->state = AVDTP_SESSION_STATE_DISCONNECTED;
@@ -2490,6 +2471,8 @@ static void auth_cb(DBusError *derr, void *user_data)
struct avdtp *session = user_data;
GError *err = NULL;

+ avdtp_unref(session);
+
if (derr && dbus_error_is_set(derr)) {
error("Access denied: %s", derr->message);
connection_lost(session, EACCES);
@@ -2578,10 +2561,12 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
auth_cb, session);
if (session->auth_id == 0) {
avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
- avdtp_unref(session);
goto drop;
}

+ /* Disable disconnect timer while authorizing */
+ avdtp_ref(session);
+
dev->auto_connect = auto_connect;

return;
@@ -3949,23 +3934,12 @@ proceed:
void avdtp_exit(const bdaddr_t *src)
{
struct avdtp_server *server;
- GSList *l;

server = find_server(servers, src);
if (!server)
return;

- l = server->sessions;
- while (l) {
- struct avdtp *session = l->data;
-
- l = l->next;
- /* value of l pointer should be updated before invoking
- * connection_lost since it internally uses avdtp_unref
- * which operates on server->session list as well
- */
- connection_lost(session, -ECONNABORTED);
- }
+ g_slist_free_full(server->sessions, avdtp_free);

servers = g_slist_remove(servers, server);

--
1.7.11.4



2012-10-18 14:57:19

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ] AVDTP: Do not keep a internal reference

Hi Ludek,

On Thu, Oct 18, 2012 at 5:20 PM, Ludek Finstrle <[email protected]> wrote:
> Hello Luiz,
>
> Wed, Oct 17, 2012 at 02:49:32PM +0300, Luiz Augusto von Dentz napsal(a):
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> Don't initialize reference with 1, instead always start disconnect timer
>> when reference drops to 0, so in case nobody reclaims the session it
>> automatically disconnect after 1 second and frees the memory.
>> ---
>> audio/avdtp.c | 220 ++++++++++++++++++++++++++--------------------------------
>> 1 file changed, 97 insertions(+), 123 deletions(-)
>>
>> diff --git a/audio/avdtp.c b/audio/avdtp.c
>> index bd91cb6..6bfeb9b 100644
>> --- a/audio/avdtp.c
>> +++ b/audio/avdtp.c
>> @@ -387,8 +387,7 @@ struct avdtp_stream {
>> /* Structure describing an AVDTP connection between two devices */
>>
>> struct avdtp {
>> - int ref;
>> - int free_lock;
>> + unsigned int ref;
>>
>> uint16_t version;
>>
>> @@ -657,50 +656,6 @@ static gboolean stream_open_timeout(gpointer user_data)
>> return FALSE;
>> }
>>
>> -static gboolean disconnect_timeout(gpointer user_data)
>> -{
>> - struct avdtp *session = user_data;
>> - struct audio_device *dev;
>> - gboolean stream_setup;
>> -
>> - session->dc_timer = 0;
>> - stream_setup = session->stream_setup;
>> - session->stream_setup = FALSE;
>> -
>> - dev = manager_get_device(&session->server->src, &session->dst, FALSE);
>> -
>> - if (dev && dev->sink && stream_setup)
>> - sink_setup_stream(dev->sink, session);
>> - else if (dev && dev->source && stream_setup)
>> - source_setup_stream(dev->source, session);
>> - else
>> - connection_lost(session, ETIMEDOUT);
>> -
>> - return FALSE;
>> -}
>> -
>> -static void remove_disconnect_timer(struct avdtp *session)
>> -{
>> - g_source_remove(session->dc_timer);
>> - session->dc_timer = 0;
>> - session->stream_setup = FALSE;
>> -}
>> -
>> -static void set_disconnect_timer(struct avdtp *session)
>> -{
>> - if (session->dc_timer)
>> - remove_disconnect_timer(session);
>> -
>> - if (session->device_disconnect) {
>> - session->dc_timer = g_idle_add(disconnect_timeout, session);
>> - return;
>> - }
>> -
>> - session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
>> - disconnect_timeout,
>> - session);
>> -}
>> -
>> void avdtp_error_init(struct avdtp_error *err, uint8_t category, int id)
>> {
>> err->category = category;
>> @@ -780,8 +735,9 @@ static void avdtp_set_state(struct avdtp *session,
>> }
>> }
>>
>> -static void stream_free(struct avdtp_stream *stream)
>> +static void stream_free(void *data)
>> {
>> + struct avdtp_stream *stream = data;
>> struct avdtp_remote_sep *rsep;
>>
>> stream->lsep->info.inuse = 0;
>> @@ -1144,37 +1100,42 @@ static int avdtp_cancel_authorization(struct avdtp *session)
>> return err;
>>
>> session->auth_id = 0;
>> + avdtp_unref(session);
>>
>> return 0;
>> }
>>
>> -static void connection_lost(struct avdtp *session, int err)
>> +static void sep_free(gpointer data)
>> {
>> - char address[18];
>> + struct avdtp_remote_sep *sep = data;
>>
>> - ba2str(&session->dst, address);
>> - DBG("Disconnected from %s", address);
>> + g_slist_free_full(sep->caps, g_free);
>> + g_free(sep);
>> +}
>>
>> - if (err != EACCES)
>> - avdtp_cancel_authorization(session);
>> +static void remove_disconnect_timer(struct avdtp *session)
>> +{
>> + g_source_remove(session->dc_timer);
>> + session->dc_timer = 0;
>> + session->stream_setup = FALSE;
>> +}
>>
>> - session->free_lock = 1;
>> +static void avdtp_free(void *data)
>> +{
>> + struct avdtp *session = data;
>>
>> - finalize_discovery(session, err);
>> + DBG("%p", session);
>>
>> - g_slist_foreach(session->streams, (GFunc) release_stream, session);
>> - session->streams = NULL;
>> + g_slist_free_full(session->streams, stream_free);
>>
>> - session->free_lock = 0;
>> + if (session->state != AVDTP_SESSION_STATE_DISCONNECTED)
>> + avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
>>
>> if (session->io) {
>> g_io_channel_shutdown(session->io, FALSE, NULL);
>> g_io_channel_unref(session->io);
>> - session->io = NULL;
>> }
>>
>> - avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
>> -
>> if (session->io_id) {
>> g_source_remove(session->io_id);
>> session->io_id = 0;
>> @@ -1183,69 +1144,95 @@ static void connection_lost(struct avdtp *session, int err)
>> if (session->dc_timer)
>> remove_disconnect_timer(session);
>>
>> - if (session->ref != 1)
>> - error("connection_lost: ref count not 1 after all callbacks");
>> - else
>> - avdtp_unref(session);
>> + avdtp_cancel_authorization(session);
>> +
>> + if (session->req)
>> + pending_req_free(session->req);
>> +
>> + g_slist_free_full(session->seps, sep_free);
>> +
>> + g_free(session->buf);
>> +
>> + g_free(session);
>> }
>>
>> -static void sep_free(gpointer data)
>> +static gboolean disconnect_timeout(gpointer user_data)
>> {
>> - struct avdtp_remote_sep *sep = data;
>> + struct avdtp *session = user_data;
>> + struct audio_device *dev;
>> + gboolean stream_setup;
>>
>> - g_slist_free_full(sep->caps, g_free);
>> - g_free(sep);
>> + session->dc_timer = 0;
>> +
>> + if (session->ref == 0) {
>> + struct avdtp_server *server = session->server;
>> +
>> + server->sessions = g_slist_remove(server->sessions, session);
>> + avdtp_free(session);
>> + return FALSE;
>> + }
>> +
>> + stream_setup = session->stream_setup;
>> + session->stream_setup = FALSE;
>> +
>> + dev = manager_get_device(&session->server->src, &session->dst, FALSE);
>> +
>> + if (dev && dev->sink && stream_setup)
>> + sink_setup_stream(dev->sink, session);
>> + else if (dev && dev->source && stream_setup)
>> + source_setup_stream(dev->source, session);
>> + else
>> + connection_lost(session, ETIMEDOUT);
>> +
>> + return FALSE;
>> }
>>
>> -void avdtp_unref(struct avdtp *session)
>> +static void set_disconnect_timer(struct avdtp *session)
>> {
>> - struct avdtp_server *server;
>> + if (session->dc_timer)
>> + remove_disconnect_timer(session);
>>
>> - if (!session)
>> + if (session->device_disconnect) {
>> + session->dc_timer = g_idle_add(disconnect_timeout, session);
>> return;
>> + }
>>
>> - session->ref--;
>> -
>> - DBG("%p: ref=%d", session, session->ref);
>> + session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
>> + disconnect_timeout,
>> + session);
>> +}
>>
>> - if (session->ref == 1) {
>> - if (session->state == AVDTP_SESSION_STATE_CONNECTING &&
>> - session->io) {
>> - avdtp_cancel_authorization(session);
>> - g_io_channel_shutdown(session->io, TRUE, NULL);
>> - g_io_channel_unref(session->io);
>> - session->io = NULL;
>> - avdtp_set_state(session,
>> - AVDTP_SESSION_STATE_DISCONNECTED);
>> - }
>> +static void connection_lost(struct avdtp *session, int err)
>> +{
>> + char address[18];
>>
>> - if (session->io)
>> - set_disconnect_timer(session);
>> - else if (!session->free_lock) /* Drop the local ref if we
>> - aren't connected */
>> - session->ref--;
>> - }
>> + ba2str(&session->dst, address);
>> + DBG("Disconnected from %s", address);
>>
>> - if (session->ref > 0)
>> - return;
>> + if (err != EACCES)
>> + avdtp_cancel_authorization(session);
>>
>> - server = session->server;
>> + g_slist_foreach(session->streams, (GFunc) release_stream, session);
>> + session->streams = NULL;
>>
>> - DBG("%p: freeing session and removing from list", session);
>> + finalize_discovery(session, err);
>>
>> - if (session->dc_timer)
>> - remove_disconnect_timer(session);
>> + avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
>> +}
>>
>> - server->sessions = g_slist_remove(server->sessions, session);
>> +void avdtp_unref(struct avdtp *session)
>> +{
>> + if (!session)
>> + return;
>>
>> - if (session->req)
>> - pending_req_free(session->req);
>> + session->ref--;
>>
>> - g_slist_free_full(session->seps, sep_free);
>> + DBG("%p: ref=%d", session, session->ref);
>>
>> - g_free(session->buf);
>> + if (session->ref > 0)
>> + return;
>>
>> - g_free(session);
>> + set_disconnect_timer(session);
>> }
>>
>> struct avdtp *avdtp_ref(struct avdtp *session)
>> @@ -2231,12 +2218,6 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
>> goto failed;
>> }
>>
>> - if (session->ref == 1 && !session->streams && !session->req)
>> - set_disconnect_timer(session);
>> -
>> - if (session->streams && session->dc_timer)
>> - remove_disconnect_timer(session);
>> -
>> if (session->req && session->req->collided) {
>> DBG("Collision detected");
>> goto next;
>> @@ -2383,7 +2364,7 @@ static struct avdtp *avdtp_get_internal(const bdaddr_t *src, const bdaddr_t *dst
>>
>> session->server = server;
>> bacpy(&session->dst, dst);
>> - session->ref = 1;
>> + set_disconnect_timer(session);
>> /* We don't use avdtp_set_state() here since this isn't a state change
>> * but just setting of the initial state */
>> session->state = AVDTP_SESSION_STATE_DISCONNECTED;
>> @@ -2490,6 +2471,8 @@ static void auth_cb(DBusError *derr, void *user_data)
>> struct avdtp *session = user_data;
>> GError *err = NULL;
>>
>> + avdtp_unref(session);
>> +
>> if (derr && dbus_error_is_set(derr)) {
>> error("Access denied: %s", derr->message);
>> connection_lost(session, EACCES);
>> @@ -2578,10 +2561,12 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
>> auth_cb, session);
>> if (session->auth_id == 0) {
>> avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
>> - avdtp_unref(session);
>> goto drop;
>> }
>>
>> + /* Disable disconnect timer while authorizing */
>> + avdtp_ref(session);
>> +
>> dev->auto_connect = auto_connect;
>>
>> return;
>> @@ -3949,23 +3934,12 @@ proceed:
>> void avdtp_exit(const bdaddr_t *src)
>> {
>> struct avdtp_server *server;
>> - GSList *l;
>>
>> server = find_server(servers, src);
>> if (!server)
>> return;
>>
>> - l = server->sessions;
>> - while (l) {
>> - struct avdtp *session = l->data;
>> -
>> - l = l->next;
>> - /* value of l pointer should be updated before invoking
>> - * connection_lost since it internally uses avdtp_unref
>> - * which operates on server->session list as well
>> - */
>> - connection_lost(session, -ECONNABORTED);
>> - }
>> + g_slist_free_full(server->sessions, avdtp_free);
>>
>> servers = g_slist_remove(servers, server);
>>
>> --
>> 1.7.11.4
>
> I tried this patch (againist patched 4.99) and it contains bug.
>
> What happens:
> I try to connect from N900 bluez-4.99 to RHEL 6 bleuz-4.66 using:
> dbus-send --system --print-reply --dest=org.bluez /org/bluez/`pidof bluetoothd`/hci0/dev_<BT ID> org.bluez.AudioSource.Connect
>
> Until first first successful connect the RHEL machine asked me to Grant/Reject the conneciton.
> But after first connect/disconnect it stop asking me and the connection failed.
>
> I could connect/disconnect several times prior applying the patch (if it doesn't crash bluetoothd).
>
> I'll try to find the reason. I just want to give you the info.

We would need to know the PulseAudio version as well, in fact this
could be because the old unix socket IPC is in use in N900 and that
may take much more time to PulseAudio to notice the connection so I
don't think this is a valid use case, things have changed quite a bit
since N900 times. In the other hand with Media API we got PA involved
pretty early and once the transport object is create it will keep a
reference to the AVDTP session.

--
Luiz Augusto von Dentz

2012-10-18 14:20:11

by Ludek Finstrle

[permalink] [raw]
Subject: Re: [PATCH BlueZ] AVDTP: Do not keep a internal reference

Hello Luiz,

Wed, Oct 17, 2012 at 02:49:32PM +0300, Luiz Augusto von Dentz napsal(a):
> From: Luiz Augusto von Dentz <[email protected]>
>
> Don't initialize reference with 1, instead always start disconnect timer
> when reference drops to 0, so in case nobody reclaims the session it
> automatically disconnect after 1 second and frees the memory.
> ---
> audio/avdtp.c | 220 ++++++++++++++++++++++++++--------------------------------
> 1 file changed, 97 insertions(+), 123 deletions(-)
>
> diff --git a/audio/avdtp.c b/audio/avdtp.c
> index bd91cb6..6bfeb9b 100644
> --- a/audio/avdtp.c
> +++ b/audio/avdtp.c
> @@ -387,8 +387,7 @@ struct avdtp_stream {
> /* Structure describing an AVDTP connection between two devices */
>
> struct avdtp {
> - int ref;
> - int free_lock;
> + unsigned int ref;
>
> uint16_t version;
>
> @@ -657,50 +656,6 @@ static gboolean stream_open_timeout(gpointer user_data)
> return FALSE;
> }
>
> -static gboolean disconnect_timeout(gpointer user_data)
> -{
> - struct avdtp *session = user_data;
> - struct audio_device *dev;
> - gboolean stream_setup;
> -
> - session->dc_timer = 0;
> - stream_setup = session->stream_setup;
> - session->stream_setup = FALSE;
> -
> - dev = manager_get_device(&session->server->src, &session->dst, FALSE);
> -
> - if (dev && dev->sink && stream_setup)
> - sink_setup_stream(dev->sink, session);
> - else if (dev && dev->source && stream_setup)
> - source_setup_stream(dev->source, session);
> - else
> - connection_lost(session, ETIMEDOUT);
> -
> - return FALSE;
> -}
> -
> -static void remove_disconnect_timer(struct avdtp *session)
> -{
> - g_source_remove(session->dc_timer);
> - session->dc_timer = 0;
> - session->stream_setup = FALSE;
> -}
> -
> -static void set_disconnect_timer(struct avdtp *session)
> -{
> - if (session->dc_timer)
> - remove_disconnect_timer(session);
> -
> - if (session->device_disconnect) {
> - session->dc_timer = g_idle_add(disconnect_timeout, session);
> - return;
> - }
> -
> - session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
> - disconnect_timeout,
> - session);
> -}
> -
> void avdtp_error_init(struct avdtp_error *err, uint8_t category, int id)
> {
> err->category = category;
> @@ -780,8 +735,9 @@ static void avdtp_set_state(struct avdtp *session,
> }
> }
>
> -static void stream_free(struct avdtp_stream *stream)
> +static void stream_free(void *data)
> {
> + struct avdtp_stream *stream = data;
> struct avdtp_remote_sep *rsep;
>
> stream->lsep->info.inuse = 0;
> @@ -1144,37 +1100,42 @@ static int avdtp_cancel_authorization(struct avdtp *session)
> return err;
>
> session->auth_id = 0;
> + avdtp_unref(session);
>
> return 0;
> }
>
> -static void connection_lost(struct avdtp *session, int err)
> +static void sep_free(gpointer data)
> {
> - char address[18];
> + struct avdtp_remote_sep *sep = data;
>
> - ba2str(&session->dst, address);
> - DBG("Disconnected from %s", address);
> + g_slist_free_full(sep->caps, g_free);
> + g_free(sep);
> +}
>
> - if (err != EACCES)
> - avdtp_cancel_authorization(session);
> +static void remove_disconnect_timer(struct avdtp *session)
> +{
> + g_source_remove(session->dc_timer);
> + session->dc_timer = 0;
> + session->stream_setup = FALSE;
> +}
>
> - session->free_lock = 1;
> +static void avdtp_free(void *data)
> +{
> + struct avdtp *session = data;
>
> - finalize_discovery(session, err);
> + DBG("%p", session);
>
> - g_slist_foreach(session->streams, (GFunc) release_stream, session);
> - session->streams = NULL;
> + g_slist_free_full(session->streams, stream_free);
>
> - session->free_lock = 0;
> + if (session->state != AVDTP_SESSION_STATE_DISCONNECTED)
> + avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
>
> if (session->io) {
> g_io_channel_shutdown(session->io, FALSE, NULL);
> g_io_channel_unref(session->io);
> - session->io = NULL;
> }
>
> - avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
> -
> if (session->io_id) {
> g_source_remove(session->io_id);
> session->io_id = 0;
> @@ -1183,69 +1144,95 @@ static void connection_lost(struct avdtp *session, int err)
> if (session->dc_timer)
> remove_disconnect_timer(session);
>
> - if (session->ref != 1)
> - error("connection_lost: ref count not 1 after all callbacks");
> - else
> - avdtp_unref(session);
> + avdtp_cancel_authorization(session);
> +
> + if (session->req)
> + pending_req_free(session->req);
> +
> + g_slist_free_full(session->seps, sep_free);
> +
> + g_free(session->buf);
> +
> + g_free(session);
> }
>
> -static void sep_free(gpointer data)
> +static gboolean disconnect_timeout(gpointer user_data)
> {
> - struct avdtp_remote_sep *sep = data;
> + struct avdtp *session = user_data;
> + struct audio_device *dev;
> + gboolean stream_setup;
>
> - g_slist_free_full(sep->caps, g_free);
> - g_free(sep);
> + session->dc_timer = 0;
> +
> + if (session->ref == 0) {
> + struct avdtp_server *server = session->server;
> +
> + server->sessions = g_slist_remove(server->sessions, session);
> + avdtp_free(session);
> + return FALSE;
> + }
> +
> + stream_setup = session->stream_setup;
> + session->stream_setup = FALSE;
> +
> + dev = manager_get_device(&session->server->src, &session->dst, FALSE);
> +
> + if (dev && dev->sink && stream_setup)
> + sink_setup_stream(dev->sink, session);
> + else if (dev && dev->source && stream_setup)
> + source_setup_stream(dev->source, session);
> + else
> + connection_lost(session, ETIMEDOUT);
> +
> + return FALSE;
> }
>
> -void avdtp_unref(struct avdtp *session)
> +static void set_disconnect_timer(struct avdtp *session)
> {
> - struct avdtp_server *server;
> + if (session->dc_timer)
> + remove_disconnect_timer(session);
>
> - if (!session)
> + if (session->device_disconnect) {
> + session->dc_timer = g_idle_add(disconnect_timeout, session);
> return;
> + }
>
> - session->ref--;
> -
> - DBG("%p: ref=%d", session, session->ref);
> + session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
> + disconnect_timeout,
> + session);
> +}
>
> - if (session->ref == 1) {
> - if (session->state == AVDTP_SESSION_STATE_CONNECTING &&
> - session->io) {
> - avdtp_cancel_authorization(session);
> - g_io_channel_shutdown(session->io, TRUE, NULL);
> - g_io_channel_unref(session->io);
> - session->io = NULL;
> - avdtp_set_state(session,
> - AVDTP_SESSION_STATE_DISCONNECTED);
> - }
> +static void connection_lost(struct avdtp *session, int err)
> +{
> + char address[18];
>
> - if (session->io)
> - set_disconnect_timer(session);
> - else if (!session->free_lock) /* Drop the local ref if we
> - aren't connected */
> - session->ref--;
> - }
> + ba2str(&session->dst, address);
> + DBG("Disconnected from %s", address);
>
> - if (session->ref > 0)
> - return;
> + if (err != EACCES)
> + avdtp_cancel_authorization(session);
>
> - server = session->server;
> + g_slist_foreach(session->streams, (GFunc) release_stream, session);
> + session->streams = NULL;
>
> - DBG("%p: freeing session and removing from list", session);
> + finalize_discovery(session, err);
>
> - if (session->dc_timer)
> - remove_disconnect_timer(session);
> + avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
> +}
>
> - server->sessions = g_slist_remove(server->sessions, session);
> +void avdtp_unref(struct avdtp *session)
> +{
> + if (!session)
> + return;
>
> - if (session->req)
> - pending_req_free(session->req);
> + session->ref--;
>
> - g_slist_free_full(session->seps, sep_free);
> + DBG("%p: ref=%d", session, session->ref);
>
> - g_free(session->buf);
> + if (session->ref > 0)
> + return;
>
> - g_free(session);
> + set_disconnect_timer(session);
> }
>
> struct avdtp *avdtp_ref(struct avdtp *session)
> @@ -2231,12 +2218,6 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
> goto failed;
> }
>
> - if (session->ref == 1 && !session->streams && !session->req)
> - set_disconnect_timer(session);
> -
> - if (session->streams && session->dc_timer)
> - remove_disconnect_timer(session);
> -
> if (session->req && session->req->collided) {
> DBG("Collision detected");
> goto next;
> @@ -2383,7 +2364,7 @@ static struct avdtp *avdtp_get_internal(const bdaddr_t *src, const bdaddr_t *dst
>
> session->server = server;
> bacpy(&session->dst, dst);
> - session->ref = 1;
> + set_disconnect_timer(session);
> /* We don't use avdtp_set_state() here since this isn't a state change
> * but just setting of the initial state */
> session->state = AVDTP_SESSION_STATE_DISCONNECTED;
> @@ -2490,6 +2471,8 @@ static void auth_cb(DBusError *derr, void *user_data)
> struct avdtp *session = user_data;
> GError *err = NULL;
>
> + avdtp_unref(session);
> +
> if (derr && dbus_error_is_set(derr)) {
> error("Access denied: %s", derr->message);
> connection_lost(session, EACCES);
> @@ -2578,10 +2561,12 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
> auth_cb, session);
> if (session->auth_id == 0) {
> avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
> - avdtp_unref(session);
> goto drop;
> }
>
> + /* Disable disconnect timer while authorizing */
> + avdtp_ref(session);
> +
> dev->auto_connect = auto_connect;
>
> return;
> @@ -3949,23 +3934,12 @@ proceed:
> void avdtp_exit(const bdaddr_t *src)
> {
> struct avdtp_server *server;
> - GSList *l;
>
> server = find_server(servers, src);
> if (!server)
> return;
>
> - l = server->sessions;
> - while (l) {
> - struct avdtp *session = l->data;
> -
> - l = l->next;
> - /* value of l pointer should be updated before invoking
> - * connection_lost since it internally uses avdtp_unref
> - * which operates on server->session list as well
> - */
> - connection_lost(session, -ECONNABORTED);
> - }
> + g_slist_free_full(server->sessions, avdtp_free);
>
> servers = g_slist_remove(servers, server);
>
> --
> 1.7.11.4

I tried this patch (againist patched 4.99) and it contains bug.

What happens:
I try to connect from N900 bluez-4.99 to RHEL 6 bleuz-4.66 using:
dbus-send --system --print-reply --dest=org.bluez /org/bluez/`pidof bluetoothd`/hci0/dev_<BT ID> org.bluez.AudioSource.Connect

Until first first successful connect the RHEL machine asked me to Grant/Reject the conneciton.
But after first connect/disconnect it stop asking me and the connection failed.

I could connect/disconnect several times prior applying the patch (if it doesn't crash bluetoothd).

I'll try to find the reason. I just want to give you the info.

Luf