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@xxxxxx 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
