[PATCH BlueZ v2 0/4] test-vcp: Fix test failing in some environments

[PATCH BlueZ v2 0/4] test-vcp: Fix test failing in some environments

[Kirill Samburskiy]

This patch fixes test-vcp failing on some environments. It is done by
shutting down tester IO before test teardown and registering detach
callbacks to ensure proper cleanup. Similar changes are introduced to
test-micp to avoid similar problems in the future.

---
Note: this is the second version of the patch, the original discussion
can be found at:
https://lore.kernel.org/all/20250619153120.126315-1-k.samburskiy@omp.ru
See patch changelog below for changes in the new version.

After updating our bluez source code from v5.72 to v5.83 we noticed
that one of the tests (test-vcp) no longer passes on all environments
(specifically when building for x86_64 or arm64 architectures). This
patch resolves the problem, enabling all tests to pass. Here is a
short extract from test logs:

```
AICS/SR/CP/BV-01-C - init
AICS/SR/CP/BV-01-C - setup
AICS/SR/CP/BV-01-C - setup complete
AICS/SR/CP/BV-01-C - run
gatt_notify_cb: Failed to send notification
ERROR:src/shared/tester.c:981:test_io_recv: assertion failed (len == iov->iov_len): (5 == 6)
```

The reason this test was failing is incomplete test teardown.
Specifically, bt_vcp instances created by vcp_get_session function
are not freed and, more importantly, not removed from sessions queue
(both function and queue are found in shared/vcp.c file).

When a new test case is started, vcp_get_session function may be called
at some point. This function looks up session object using current
bt_att object as key. Each test case creates its own bt_att instance,
however in our case bt_att is always allocated at the same memory
address. This leads to vcp_get_session function looking up session
object belonging to the previous test case instead of creating a new
bt_vcp instance (since both current and previous test cases allocated
memory for bt_att object at the same address).

Getting the wrong session object leads to using wrong gatt_db, which
then uses the wrong user_data for CCC callbacks, ultimately leading
to gatt_notify_cb function from test-vcp.c getting incorrect test_data
pointer. Finally gatt_notify_cb attempts to send a notification using
an already freed bt_gatt_server instance, which unsurprisingly fails,
causing expected data to not be written into tester IO channel.

This patch fixes the issue by doing two things. First, it shuts down
tester IO as a part of test teardown, triggering disconnection
callbacks in bt_att object. One of these callbacks is registered in
vcp_get_session function, specifically vcp_disconnected function.
This function detaches bt_vcp instance (removes from sessions queue)
and triggers *_remote_client_detached callbacks. The second part of the
fix is registering vcp remote client callbacks using bt_vcp_register
function, with vcp_client_detached function responsible for unrefing
(and freeing) the detached bt_vcp instance. Since the instance is now
removed from sessions queue, vcp_get_session function can no longer
look up a wrong object during the test, allowing it to pass.

---
v2: tester_shutdown_io is now called from test_result, thus applying to
    all tests.
    Removed bt_vcp allocation from test-vcp since it was unnecessary.
    Added bt_vcp_add_db call to ensure all relevant attributes are
    still added to gatt_db.
    Introduced similar changes to test-micp: removed unnecessary
    bt_micp allocation and added detach callback for freeing bt_micp
    instance created by micp_get_session.
    In shared/bap, NULL attach or detach callbacks are now ignored to
    ensure test-bap does not fail due to introduction of IO shutdown.
---
Kirill Samburskiy (4):
  shared/bap: ignore NULL attach/detach callbacks
  test-vcp: remove unnecessary bt_vcp allocation
  test-micp: remove unnecessary bt_micp allocation
  shared/tester: shutdown tester IO before test teardown

 src/shared/bap.c    |  6 ++++++
 src/shared/tester.c |  8 ++++++++
 src/shared/tester.h |  1 +
 unit/test-micp.c    | 18 ++++++++++++++----
 unit/test-vcp.c     | 19 +++++++++++++++----
 5 files changed, 44 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH BlueZ v2 1/4] shared/bap: ignore NULL attach/detach callbacks

[Kirill Samburskiy]

Allow registering NULL attach/detach callbacks with bt_bap_register
for cases when one of callbacks is not needed, e.g. in tests.
---
 src/shared/bap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index f0c6f6485..76340d565 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -4415,6 +4415,9 @@ static void bap_detached(void *data, void *user_data)
 	struct bt_bap_cb *cb = data;
 	struct bt_bap *bap = user_data;
 
+	if (!cb->detached)
+		return;
+
 	cb->detached(bap, cb->user_data);
 }
 
@@ -4499,6 +4502,9 @@ static void bap_attached(void *data, void *user_data)
 	struct bt_bap_cb *cb = data;
 	struct bt_bap *bap = user_data;
 
+	if (!cb->attached)
+		return;
+
 	cb->attached(bap, cb->user_data);
 }
 
-- 
2.34.1

RE: test-vcp: Fix test failing in some environments

[bluez.test.bot]

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=974291

---Test result---

Test Summary:
CheckPatch                    PENDING   0.44 seconds
GitLint                       PENDING   0.40 seconds
BuildEll                      PASS      20.10 seconds
BluezMake                     PASS      2643.80 seconds
MakeCheck                     PASS      20.88 seconds
MakeDistcheck                 PASS      196.59 seconds
CheckValgrind                 PASS      274.06 seconds
CheckSmatch                   WARNING   306.77 seconds
bluezmakeextell               PASS      127.66 seconds
IncrementalBuild              PENDING   0.39 seconds
ScanBuild                     PASS      918.78 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

[PATCH BlueZ v2 2/4] test-vcp: remove unnecessary bt_vcp allocation

[Kirill Samburskiy]

Do not create bt_vcp in test_server since it is not necessary for this
test. Also register bt_vcp detached callback to unref and free
objects created by vcp_get_session.
---
 unit/test-vcp.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/unit/test-vcp.c b/unit/test-vcp.c
index 6a61ea2c4..2a6e77eb0 100644
--- a/unit/test-vcp.c
+++ b/unit/test-vcp.c
@@ -33,11 +33,11 @@
 
 struct test_data {
 	struct gatt_db *db;
-	struct bt_vcp *vcp;
 	struct bt_gatt_server *server;
 	struct queue *ccc_states;
 	size_t iovcnt;
 	struct iovec *iov;
+	unsigned int vcp_id;
 };
 
 struct notify {
@@ -82,7 +82,6 @@ static void test_teardown(const void *user_data)
 {
 	struct test_data *data = (void *)user_data;
 
-	bt_vcp_unref(data->vcp);
 	bt_gatt_server_unref(data->server);
 	util_iov_free(data->iov, data->iovcnt);
 
@@ -90,6 +89,7 @@ static void test_teardown(const void *user_data)
 
 	queue_destroy(data->ccc_states, free);
 
+	bt_vcp_unregister(data->vcp_id);
 	tester_teardown_complete();
 }
 
@@ -163,6 +163,15 @@ done:
 							sizeof(value));
 }
 
+static void vcp_client_attached(struct bt_vcp *vcp, void *user_data)
+{
+}
+
+static void vcp_client_detached(struct bt_vcp *vcp, void *user_data)
+{
+	bt_vcp_unref(vcp);
+}
+
 static void test_server(const void *user_data)
 {
 	struct test_data *data = (void *)user_data;
@@ -185,8 +194,10 @@ static void test_server(const void *user_data)
 	gatt_db_ccc_register(data->db, gatt_ccc_read_cb, NULL,
 					gatt_notify_cb, data);
 
-	data->vcp = bt_vcp_new(data->db, NULL);
-	g_assert(data->vcp);
+	bt_vcp_add_db(data->db);
+
+	data->vcp_id = bt_vcp_register(vcp_client_attached,
+						vcp_client_detached, NULL);
 
 	data->server = bt_gatt_server_new(data->db, att, 64, 0);
 	g_assert(data->server);
-- 
2.34.1

[PATCH BlueZ v2 3/4] test-micp: remove unnecessary bt_micp allocation

[Kirill Samburskiy]

Do not create bt_micp in test_server since it is not necessary for this
test. Also register bt_micp detached callback to unref and free
objects created by micp_get_session.
---
 unit/test-micp.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/unit/test-micp.c b/unit/test-micp.c
index a7fc7fb32..87fde8ed0 100644
--- a/unit/test-micp.c
+++ b/unit/test-micp.c
@@ -33,12 +33,12 @@
 
 struct test_data_mics {
 	struct gatt_db *db;
-	struct bt_micp *micp;
 	struct bt_gatt_server *server;
 	struct bt_gatt_client *client;
 	struct queue *ccc_states;
 	size_t iovcnt;
 	struct iovec *iov;
+	unsigned int micp_id;
 };
 
 struct test_data_micp {
@@ -98,10 +98,10 @@ static void test_teardown_mics(const void *user_data)
 {
 	struct test_data_mics *data = (void *)user_data;
 
-	bt_micp_unref(data->micp);
 	bt_gatt_server_unref(data->server);
 	util_iov_free(data->iov, data->iovcnt);
 	gatt_db_unref(data->db);
+	bt_micp_unregister(data->micp_id);
 
 	queue_destroy(data->ccc_states, free);
 
@@ -269,6 +269,15 @@ done:
 	gatt_db_attribute_read_result(attrib, id, ecode, value, len);
 }
 
+static void micp_attached(struct bt_micp *micp, void *user_data)
+{
+}
+
+static void micp_detached(struct bt_micp *micp, void *user_data)
+{
+	bt_micp_unref(micp);
+}
+
 static void test_server(const void *user_data)
 {
 	struct test_data_mics *data = (void *)user_data;
@@ -291,8 +300,9 @@ static void test_server(const void *user_data)
 	gatt_db_ccc_register(data->db, gatt_ccc_read_cb, NULL,
 					gatt_notify_cb, data);
 
-	data->micp = bt_micp_new(data->db, NULL);
-	g_assert(data->micp);
+	bt_micp_add_db(data->db);
+
+	data->micp_id = bt_micp_register(micp_attached, micp_detached, NULL);
 
 	data->server = bt_gatt_server_new(data->db, att, 64, 0);
 	g_assert(data->server);
-- 
2.34.1

[PATCH BlueZ v2 4/4] shared/tester: shutdown tester IO before test teardown

[Kirill Samburskiy]

Some tests may require shutdown of tester IO for proper teardown.
Add function tester_shutdown_io to accomplish that and call it
automatically when test finishes.
---
 src/shared/tester.c | 8 ++++++++
 src/shared/tester.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/src/shared/tester.c b/src/shared/tester.c
index 371ccaced..62a14d073 100644
--- a/src/shared/tester.c
+++ b/src/shared/tester.c
@@ -654,6 +654,8 @@ static void test_result(enum test_result result)
 		test->timeout_id = 0;
 	}
 
+	tester_shutdown_io();
+
 	if (test->result == TEST_RESULT_FAILED)
 		result = TEST_RESULT_FAILED;
 
@@ -1047,6 +1049,12 @@ struct io *tester_setup_io(const struct iovec *iov, int iovcnt)
 	return ios[0];
 }
 
+void tester_shutdown_io(void)
+{
+	io_shutdown(ios[0]);
+	io_shutdown(ios[1]);
+}
+
 void tester_io_send(void)
 {
 	struct test_case *test = tester_get_test();
diff --git a/src/shared/tester.h b/src/shared/tester.h
index 1f8138434..dfc1ca3a8 100644
--- a/src/shared/tester.h
+++ b/src/shared/tester.h
@@ -81,5 +81,6 @@ void tester_wait(unsigned int seconds, tester_wait_func_t func,
 							void *user_data);
 
 struct io *tester_setup_io(const struct iovec *iov, int iovcnt);
+void tester_shutdown_io(void);
 void tester_io_send(void);
 void tester_io_set_complete_func(tester_data_func_t func);
-- 
2.34.1

Re: [PATCH BlueZ v2 0/4] test-vcp: Fix test failing in some environments

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 20 Jun 2025 16:51:42 +0300 you wrote:
> This patch fixes test-vcp failing on some environments. It is done by
> shutting down tester IO before test teardown and registering detach
> callbacks to ensure proper cleanup. Similar changes are introduced to
> test-micp to avoid similar problems in the future.
> 
> ---
> Note: this is the second version of the patch, the original discussion
> can be found at:
> https://lore.kernel.org/all/20250619153120.126315-1-k.samburskiy@omp.ru
> See patch changelog below for changes in the new version.
> 
> [...]

Here is the summary with links:
  - [BlueZ,v2,1/4] shared/bap: ignore NULL attach/detach callbacks
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=71b5ea9bf373
  - [BlueZ,v2,2/4] test-vcp: remove unnecessary bt_vcp allocation
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f5a4e95d73fe
  - [BlueZ,v2,3/4] test-micp: remove unnecessary bt_micp allocation
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=659fc8fa7037
  - [BlueZ,v2,4/4] shared/tester: shutdown tester IO before test teardown
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a209d69767db

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html