All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 1/2] shared/asha: Cancel callbacks before resetting the device
@ 2026-10-15  4:55 Kyle Pepper
  2026-10-15  4:56 ` [RFC PATCH 2/2] asha: Gate streaming on the observed connection interval Kyle Pepper
  0 siblings, 1 reply; 2+ messages in thread
From: Kyle Pepper @ 2026-10-15  4:55 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Marcel Holtmann, Luiz Augusto von Dentz

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-15  4:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15  4:55 [RFC PATCH 1/2] shared/asha: Cancel callbacks before resetting the device Kyle Pepper
2026-10-15  4:56 ` [RFC PATCH 2/2] asha: Gate streaming on the observed connection interval Kyle Pepper

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.