Linux bluetooth development
 help / color / mirror / Atom feed
From: Kyle Pepper <cochcoder@proton.me>
To: linux-bluetooth@vger.kernel.org
Cc: Marcel Holtmann <marcel@holtmann.org>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Subject: [RFC PATCH 1/2] shared/asha: Cancel callbacks before resetting the device
Date: Thu, 15 Oct 2026 04:55:44 +0000	[thread overview]
Message-ID: <20261015045525.66971-1-cochcoder@proton.me> (raw)

Track and cancel the pending ACP request before releasing stream state.
Detach state callbacks before invoking an owner that may remove the device.
Send STOP when canceling a START that is already queued.

Detach set membership before clearing its identity, retain replacement
members and clear notification registration on reset.

Fixes: 8e4bece63ce9 ("src/shared: Add initial implementation for an ASHA profile")
Fixes: f44d3e08e9a1 ("shared/asha: Add support for other side update")
Assisted-by: OpenCode:gpt-6.1-sol
---
First of two new patches. Apply on BlueZ dd9d5ba1fa4f with the already-sent
STATUS v2 and STOP changes, in that order. These prerequisites are not resent:
https://lore.kernel.org/linux-bluetooth/20261013222556.513513-1-cochcoder@proton.me/
https://lore.kernel.org/linux-bluetooth/20261013034345.513789-1-cochcoder@proton.me/

This patch handles pending ACP request/callback ownership, cancellation and
set/notification reset. Patch 2 uses these lifecycle operations to gate ASHA
startup and stop acquired streams after incompatible completed intervals.

The prerequisite alone passes its ASHA sanitizer fixtures. The combined
standalone source passes 38/38 strict-maintainer ASan/UBSan and 38/38 ordinary
LTO unit binaries. The full strict-maintainer plus LTO combination retains
documented baseline GATT/monitor errors; it is not claimed passing.

The Arch integration was built in a clean chroot and is reported working on
hardware. That trial also includes other existing runtime fixes; it is not
isolated attribution or coverage of every cancellation/teardown interleaving.
No remote CI, severity assessment or proof of all surrounding lifetimes is claimed.

 src/shared/asha.c         |  63 +++++++++++-------
 src/shared/asha.h         |   1 +
 unit/test-asha-interval.c | 137 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 177 insertions(+), 24 deletions(-)

diff --git a/src/shared/asha.c b/src/shared/asha.c
index f73bbce1d5..e6862e8423 100644
--- a/src/shared/asha.c
+++ b/src/shared/asha.c
@@ -106,10 +106,10 @@ static void update_asha_set(struct bt_asha *asha, bool connected)
 			return;
 		}
 
-		if (asha->right_side && set->right) {
+		if (asha->right_side && set->right == asha) {
 			set->right = NULL;
 			DBG("Right side unregistered for ASHA set");
-		} else if (!asha->right_side && set->left) {
+		} else if (!asha->right_side && set->left == asha) {
 			set->left = NULL;
 			DBG("Left side unregistered for ASHA set");
 		}
@@ -163,9 +163,13 @@ struct bt_asha *bt_asha_new(void)
 
 void bt_asha_reset(struct bt_asha *asha)
 {
+	bt_asha_state_reset(asha);
+	update_asha_set(asha, false);
+
 	if (asha->status_notify_id) {
 		bt_gatt_client_unregister_notify(asha->client,
 						asha->status_notify_id);
+		asha->status_notify_id = 0;
 	}
 
 	gatt_db_unref(asha->db);
@@ -174,19 +178,20 @@ void bt_asha_reset(struct bt_asha *asha)
 	bt_gatt_client_unref(asha->client);
 	asha->client = NULL;
 
-	bt_asha_state_reset(asha);
-
 	asha->psm = 0;
 	memset(asha->hisyncid, 0, sizeof(asha->hisyncid));
 
 	asha->attach_cb = NULL;
 	asha->attach_cb_data = NULL;
-
-	update_asha_set(asha, false);
 }
 
 void bt_asha_state_reset(struct bt_asha *asha)
 {
+	if (asha->acp_id) {
+		bt_gatt_client_cancel(asha->client, asha->acp_id);
+		asha->acp_id = 0;
+	}
+
 	asha->state = ASHA_STOPPED;
 
 	asha->state_cb = NULL;
@@ -195,33 +200,40 @@ void bt_asha_state_reset(struct bt_asha *asha)
 
 void bt_asha_free(struct bt_asha *asha)
 {
-	update_asha_set(asha, false);
-	gatt_db_unref(asha->db);
-	bt_gatt_client_unref(asha->client);
+	bt_asha_reset(asha);
 	free(asha);
 }
 
 static void asha_acp_sent(bool success, uint8_t err, void *user_data)
 {
 	struct bt_asha *asha = user_data;
+	bt_asha_cb_t callback = asha->state_cb;
+	void *data = asha->state_cb_data;
+
+	asha->acp_id = 0;
 
 	if (success) {
 		DBG("AudioControlPoint command successfully sent");
 	} else {
 		error("Failed to send AudioControlPoint command: %d", err);
 
-		if (asha->state_cb)
-			asha->state_cb(-1, asha->state_cb_data);
-
 		bt_asha_state_reset(asha);
+
+		if (callback)
+			callback(-1, data);
 	}
 }
 
 static int asha_send_acp(struct bt_asha *asha, uint8_t *cmd,
 		unsigned int len, bt_asha_cb_t cb, void *user_data)
 {
-	if (!bt_gatt_client_write_value(asha->client, asha->acp_handle, cmd,
-				len, asha_acp_sent, asha, NULL)) {
+	if (asha->acp_id)
+		bt_gatt_client_cancel(asha->client, asha->acp_id);
+
+	asha->acp_id = bt_gatt_client_write_value(asha->client,
+				asha->acp_handle, cmd, len, asha_acp_sent,
+				asha, NULL);
+	if (!asha->acp_id) {
 		error("Error writing ACP command");
 		return -1;
 	}
@@ -307,18 +319,17 @@ unsigned int bt_asha_stop(struct bt_asha *asha)
 	};
 	int ret;
 
-	if (asha->state != ASHA_STARTED)
+	if (asha->state == ASHA_STOPPED)
 		return 0;
 
-	asha->state = ASHA_STOPPED;
+	/* Cancel a pending START before queuing STOP on the same ATT link. */
+	bt_asha_state_reset(asha);
 
 	/* State is reset below without waiting for an ATT response. */
 	ret = asha_send_acp_without_response(asha, acp_stop_cmd,
 			sizeof(acp_stop_cmd));
 	asha_set_send_status(asha, false);
 
-	/* We reset our state without waiting for a response */
-	bt_asha_state_reset(asha);
 	DBG("ASHA stop done");
 
 	return ret;
@@ -468,10 +479,17 @@ static void audio_status_notify(uint16_t value_handle, const uint8_t *value,
 					uint16_t length, void *user_data)
 {
 	struct bt_asha *asha = user_data;
-	uint8_t status = *value;
+	uint8_t status;
 	/* Back these up to survive the reset paths */
 	bt_asha_cb_t state_cb = asha->state_cb;
-	bt_asha_cb_t state_cb_data = asha->state_cb_data;
+	void *state_cb_data = asha->state_cb_data;
+
+	if (length != 1)
+		return;
+
+	status = value[0];
+	asha->state_cb = NULL;
+	asha->state_cb_data = NULL;
 
 	DBG("ASHA status %u", status);
 
@@ -487,11 +505,8 @@ static void audio_status_notify(uint16_t value_handle, const uint8_t *value,
 		}
 	}
 
-	if (state_cb) {
+	if (state_cb)
 		state_cb(status, state_cb_data);
-		asha->state_cb = NULL;
-		asha->state_cb_data = NULL;
-	}
 }
 
 static void handle_characteristic(struct gatt_db_attribute *attr,
diff --git a/src/shared/asha.h b/src/shared/asha.h
index e6b2e7a0d2..cbacb5ddff 100644
--- a/src/shared/asha.h
+++ b/src/shared/asha.h
@@ -31,6 +31,7 @@ struct bt_asha {
 	uint16_t acp_handle;
 	uint16_t volume_handle;
 	unsigned int status_notify_id;
+	unsigned int acp_id;
 
 	uint16_t psm;
 	bool right_side;
diff --git a/unit/test-asha-interval.c b/unit/test-asha-interval.c
index 8cb77c07d3..5432cfb041 100644
--- a/unit/test-asha-interval.c
+++ b/unit/test-asha-interval.c
@@ -3,13 +3,136 @@
 
 /* Redirect source-level calls so LTO cannot bypass the GATT mock. */
 #define bt_gatt_client_write_without_response test_gatt_write
+#define bt_gatt_client_write_value test_gatt_request
+#define bt_gatt_client_cancel test_gatt_cancel
+#define bt_gatt_client_unref test_client_unref
+#define bt_gatt_client_unregister_notify test_unregister_notify
 #include "src/shared/asha.c"
 #undef bt_gatt_client_write_without_response
+#undef bt_gatt_client_write_value
+#undef bt_gatt_client_cancel
+#undef bt_gatt_client_unref
+#undef bt_gatt_client_unregister_notify
 #include "src/shared/tester.h"
 
 static unsigned int writes;
 static struct bt_gatt_client *failed_client;
 static uint8_t expected_interval;
+static unsigned int cancels;
+static unsigned int callbacks;
+static unsigned int unregisters;
+static unsigned int stops;
+
+void test_client_unref(struct bt_gatt_client *client)
+{
+}
+
+bool test_unregister_notify(struct bt_gatt_client *client, unsigned int id)
+{
+	assert(client && id == 7);
+	unregisters++;
+	return true;
+}
+
+unsigned int test_gatt_request(struct bt_gatt_client *client, uint16_t handle,
+			const uint8_t *value, uint16_t length,
+			bt_gatt_client_callback_t callback, void *user_data,
+			bt_gatt_client_destroy_func_t destroy)
+{
+	assert(client && handle == 0x79);
+	assert(length == 5 && value[0] == 1);
+	return 42;
+}
+
+bool test_gatt_cancel(struct bt_gatt_client *client, unsigned int id)
+{
+	assert(client && id == 42);
+	cancels++;
+	return true;
+}
+
+static void state_callback(int status, void *user_data)
+{
+	struct bt_asha *asha = user_data;
+
+	assert(asha->state == ASHA_STOPPED);
+	assert(!asha->state_cb && !asha->state_cb_data);
+	callbacks++;
+}
+
+static void test_start_cancel(const void *user_data)
+{
+	struct bt_asha asha = { .client = (void *) 1, .acp_handle = 0x79 };
+
+	cancels = callbacks = 0;
+	assert(!bt_asha_start(&asha, state_callback, &asha));
+	bt_asha_state_reset(&asha);
+	assert(cancels == 1 && !callbacks);
+	bt_asha_state_reset(&asha);
+	assert(cancels == 1);
+	assert(!bt_asha_start(&asha, state_callback, &asha));
+	asha_acp_sent(false, 1, &asha);
+	assert(callbacks == 1 && cancels == 1);
+	tester_test_passed();
+}
+
+static void test_start_stop(const void *user_data)
+{
+	struct bt_asha asha = { .client = (void *) 1, .acp_handle = 0x79 };
+	uint8_t status = 0;
+
+	cancels = callbacks = stops = 0;
+	assert(!bt_asha_start(&asha, state_callback, &asha));
+	assert(!bt_asha_stop(&asha));
+	assert(cancels == 1 && stops == 1 && !asha.state_cb);
+	audio_status_notify(1, &status, 1, &asha);
+	assert(asha.state == ASHA_STOPPED && !callbacks);
+	assert(!bt_asha_stop(&asha) && stops == 1);
+	tester_test_passed();
+}
+
+static void test_status_callback(const void *user_data)
+{
+	struct bt_asha asha = { .client = (void *) 1, .acp_handle = 0x79 };
+	uint8_t status = 1;
+
+	callbacks = 0;
+	assert(!bt_asha_start(&asha, state_callback, &asha));
+	asha_acp_sent(true, 0, &asha);
+	audio_status_notify(1, NULL, 0, &asha);
+	assert(!callbacks && asha.state_cb == state_callback);
+	audio_status_notify(1, &status, 1, &asha);
+	assert(callbacks == 1);
+	tester_test_passed();
+}
+
+static void test_pair_reset(const void *user_data)
+{
+	struct bt_asha left = { .hisyncid = { 1 } };
+	struct bt_asha right = { .hisyncid = { 1 }, .right_side = true };
+	struct bt_asha replacement = { .hisyncid = { 1 } };
+	struct bt_asha_set *set;
+
+	asha_devices = queue_new();
+	update_asha_set(&left, true);
+	update_asha_set(&right, true);
+	update_asha_set(&replacement, true);
+	bt_asha_reset(&left);
+	set = find_asha_set(&right);
+	assert(set && set->left == &replacement && set->right == &right);
+	bt_asha_reset(&replacement);
+	assert(!set->left && set->right == &right);
+	bt_asha_reset(&right);
+	assert(!asha_devices);
+
+	right.client = (void *) 1;
+	right.status_notify_id = 7;
+	unregisters = 0;
+	bt_asha_reset(&right);
+	bt_asha_reset(&right);
+	assert(unregisters == 1 && !right.status_notify_id);
+	tester_test_passed();
+}
 
 void btd_error(uint16_t index, const char *format, ...)
 {
@@ -26,6 +149,13 @@ unsigned int test_gatt_write(
 	assert(client);
 	assert(handle == 0x79);
 	assert(!signed_write);
+
+	if (length == 1) {
+		assert(value[0] == 0x02);
+		stops++;
+		return true;
+	}
+
 	assert(length == 3);
 	assert(value[0] == 0x03 && value[1] == 0x02);
 	assert(value[2] == expected_interval);
@@ -100,5 +230,12 @@ int main(int argc, char *argv[])
 {
 	tester_init(&argc, &argv);
 	tester_add("ASHA/interval/pair", NULL, NULL, test_interval, NULL);
+	tester_add("ASHA/start/cancel", NULL, NULL, test_start_cancel, NULL);
+	tester_add("ASHA/start/stop-pending", NULL, NULL,
+							test_start_stop, NULL);
+	tester_add("ASHA/start/status-callback", NULL, NULL,
+						test_status_callback, NULL);
+	tester_add("ASHA/interval/pair-reset", NULL, NULL,
+							test_pair_reset, NULL);
 	return tester_run();
 }

base-commit: dd9d5ba1fa4f682b082b1275a8c38523d95a58fd
prerequisite-patch-id: ddf57fe789414af62220e755ca04c36ec8d46260
prerequisite-patch-id: 612e58f161bb31025d30aabe6a68809255e314ba


             reply	other threads:[~2026-10-15  4:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15  4:55 Kyle Pepper [this message]
2026-10-15  4:56 ` [RFC PATCH 2/2] asha: Gate streaming on the observed connection interval Kyle Pepper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261015045525.66971-1-cochcoder@proton.me \
    --to=cochcoder@proton.me \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox