All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.