[PATCH] Fix avoid starting AVDTP disconnect timer twice

[PATCH] Fix avoid starting AVDTP disconnect timer twice

[Daniel Örstadius]

[-- Attachment #1: Type: text/plain, Size: 36 bytes --]

Patch proposal for review.

/Daniel

[-- Attachment #2: 0001-Fix-avoid-starting-AVDTP-disconnect-timer-twice.patch --]
[-- Type: text/x-patch, Size: 1371 bytes --]

From 0aad44e90585e6a7c9a59cee2cdeff00316a527c Mon Sep 17 00:00:00 2001
From: Daniel Orstadius <daniel.orstadius@nokia.com>
Date: Mon, 1 Nov 2010 12:44:25 +0200
Subject: [PATCH] Fix avoid starting AVDTP disconnect timer twice

Remove starting the timer when setting the AVDTP state to idle. If
needed, the timer should probably already have been started in
avdtp_unref when the reference count goes to one.

Since reference counting is handled in avdtp_ref and avdtp_unref, it
seems reasonable that not to inspect the count outside of those
functions.

The issue was found when using Device.Disconnect to disconnect a
headset. It was revealed by commit
c72ce0f12a8387a70a6f0109f13bd6f414f32be8.

Before the commit, the timer was removed and then started again.
After applying it, the idle callback (disconnect_timeout) is called
twice, causing a crash.
---
 audio/avdtp.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index 5e84a45..611fd7b 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -1034,8 +1034,6 @@ static void avdtp_sep_set_state(struct avdtp *session,
 		/* Remove pending commands for this stream from the queue */
 		cleanup_queue(session, stream);
 		stream_free(stream);
-		if (session->ref == 1 && !session->streams)
-			set_disconnect_timer(session);
 		break;
 	default:
 		break;
-- 
1.6.0.4

Re: [PATCH] Fix avoid starting AVDTP disconnect timer twice

[Johan Hedberg]

Hi Daniel,

On Mon, Nov 01, 2010, Daniel Örstadius wrote:
> Remove starting the timer when setting the AVDTP state to idle. If
> needed, the timer should probably already have been started in
> avdtp_unref when the reference count goes to one.
> 
> Since reference counting is handled in avdtp_ref and avdtp_unref, it
> seems reasonable that not to inspect the count outside of those
> functions.
> 
> The issue was found when using Device.Disconnect to disconnect a
> headset. It was revealed by commit
> c72ce0f12a8387a70a6f0109f13bd6f414f32be8.
> 
> Before the commit, the timer was removed and then started again.
> After applying it, the idle callback (disconnect_timeout) is called
> twice, causing a crash.

Thanks for investigating and fixing this. avdtp_unref indeed does
already take care of the timer so the call in set_state seems redundant
(in addition to being in a questionable place to begin with). The patch
has been pushed upstream.

Johan