Return-Path: From: Luiz Augusto von Dentz To: linux-bluetooth@vger.kernel.org Subject: [PATCH BlueZ v2] AVDTP: Do not keep a internal reference Date: Fri, 19 Oct 2012 12:59:23 +0300 Message-Id: <1350640763-24186-1-git-send-email-luiz.dentz@gmail.com> Sender: linux-bluetooth-owner@vger.kernel.org List-ID: From: Luiz Augusto von Dentz 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. --- v2: Fix with stream_setup flag being ignored in disconnect_timeout audio/avdtp.c | 217 +++++++++++++++++++++++++--------------------------------- 1 file changed, 94 insertions(+), 123 deletions(-) diff --git a/audio/avdtp.c b/audio/avdtp.c index bd91cb6..bca4809 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,92 @@ 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; + + 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 if (session->ref > 0) + connection_lost(session, ETIMEDOUT); + else { + struct avdtp_server *server = session->server; + + server->sessions = g_slist_remove(server->sessions, session); + avdtp_free(session); + } + + 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 +2215,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 +2361,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 +2468,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 +2558,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 +3931,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