This patch fixes test-vcp failing on some environments. It is done by manually shutting down tester IO as a part of test teardown, as well as freeing server-side bt_vcp instances. --- 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. The test teardown is now split in two functions. The reason for that is that IO disonnection callbacks are executed by main loop, thus after io_shutdown functions were executed, we need to return to main loop to let them execute before proceeding with teardown and freeing bt_att. If bt_att is freed too early, its disconnection callbacks are not going to be executed. Kirill Samburskiy (2): shared/tester: add ability to shutdown tester IO test-vcp: free server-side bt_vcp on test teardown src/shared/tester.c | 6 ++++++ src/shared/tester.h | 1 + unit/test-vcp.c | 26 ++++++++++++++++++++++++-- 3 files changed, 31 insertions(+), 2 deletions(-) -- 2.34.1
