[PATCH 00/13] Make android-tester callbacks thread safe

[PATCH 00/13] Make android-tester callbacks thread safe

[Jakub Tyszkowski]

This patch set makes all callbacks executions in HAL's notification thread being
transfered to tester's context. As test-specific callbacks are called by generic
callbacks, making the later executed in tester's main loop makes all custom
callbacks (and newly added) automatically executed in the right contex.

This is basically RFCv2 resend as PATCH with two follow-up patches.

Changes in relation to RFCv2:
  * [PATCH 12/13]: Changed daemon termination checks to non-blocking
  * [PATCH 13/13]: Added guard to execute all scheduled callbacks in current
    test's context

Best regards,

Jakub Tyszkowski (13):
  android/tester: Execute device found cbacks in main loop
  android/tester: Execute discovery state cbacks in main loop
  android/tester: Execute device properties cbacks in main loop
  android/tester: Execute adapter props cbacks in main loop
  android/tester: Execute adapter state changed cbacks in main loop
  android/tester: Execute socket cbacks in main loop
  android/tester: Execute hh connection state cbacks in main loop
  android/tester: Execute hh info cbacks in main loop
  android/tester: Execute hh protocol mode cbacks in main loop
  android/tester: Execute hh report cbacks in main loop
  android/tester: Execute hh virtual unplug cbacks in main loop
  android/tester: Non-blocking check for daemon termination
  android/tester: Add guard for late callback calls

 android/android-tester.c | 375 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 326 insertions(+), 49 deletions(-)

--
1.8.5.2

[PATCH 01/13] android/tester: Execute device found cbacks in main loop

[Jakub Tyszkowski]

Execute generic device_found_cb in tester's main loop.
---
 android/android-tester.c | 60 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index baa0115..d609323 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -117,6 +117,16 @@ struct test_data {
 	uint16_t intr_cid;
 };
 
+struct bt_cb_data {
+	bt_state_t state;
+	bt_status_t status;
+
+	bt_bdaddr_t bdaddr;
+
+	int num;
+	bt_property_t *props;
+};
+
 static char exec_dir[PATH_MAX + 1];
 
 static void mgmt_debug(const char *str, void *user_data)
@@ -695,6 +705,31 @@ static void discovery_state_changed_cb(bt_discovery_state_t state)
 	}
 }
 
+static bt_property_t *copy_properties(int num_properties,
+						bt_property_t *properties)
+{
+	int i;
+	bt_property_t *props = g_new0(bt_property_t, num_properties);
+
+	for (i = 0; i < num_properties; i++) {
+		props[i].type = properties[i].type;
+		props[i].len = properties[i].len;
+		props[i].val = g_memdup(properties[i].val, properties[i].len);
+	}
+
+	return props;
+}
+
+static void free_properties(int num_properties, bt_property_t *properties)
+{
+	int i;
+
+	for (i = 0; i < num_properties; i++)
+		g_free(properties[i].val);
+
+	g_free(properties);
+}
+
 static void discovery_device_found_cb(int num_properties,
 						bt_property_t *properties)
 {
@@ -832,15 +867,30 @@ static void remote_setprop_fail_device_found_cb(int num_properties,
 	check_expected_status(status);
 }
 
-static void device_found_cb(int num_properties, bt_property_t *properties)
+static gboolean device_found(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct bt_cb_data *cb_data = user_data;
 
-	if (data->test_init_done && test->expected_hal_cb.device_found_cb) {
-		test->expected_hal_cb.device_found_cb(num_properties,
-								properties);
-	}
+	if (data->test_init_done && test->expected_hal_cb.device_found_cb)
+		test->expected_hal_cb.device_found_cb(cb_data->num,
+								cb_data->props);
+
+	free_properties(cb_data->num, cb_data->props);
+	g_free(cb_data);
+
+	return FALSE;
+}
+
+static void device_found_cb(int num_properties, bt_property_t *properties)
+{
+	struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+	cb_data->num = num_properties;
+	cb_data->props = copy_properties(num_properties, properties);
+
+	g_idle_add(device_found, cb_data);
 }
 
 static void check_count_properties_cb(bt_status_t status, int num_properties,
-- 
1.8.5.2

[PATCH 02/13] android/tester: Execute discovery state cbacks in main loop

[Jakub Tyszkowski]

Execute generic discovery_state_changed_cb in tester's main loop.
---
 android/android-tester.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index d609323..0976081 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -695,14 +695,27 @@ static void remote_setprop_disc_state_changed_cb(bt_discovery_state_t state)
 	}
 }
 
-static void discovery_state_changed_cb(bt_discovery_state_t state)
+static gboolean discovery_state_changed(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct bt_cb_data *cb_data = user_data;
 
-	if (test && test->expected_hal_cb.discovery_state_changed_cb) {
-		test->expected_hal_cb.discovery_state_changed_cb(state);
-	}
+	if (test && test->expected_hal_cb.discovery_state_changed_cb)
+		test->expected_hal_cb.discovery_state_changed_cb(
+								cb_data->state);
+
+	g_free(cb_data);
+
+	return FALSE;
+}
+
+static void discovery_state_changed_cb(bt_discovery_state_t state)
+{
+	struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+	cb_data->state = state;
+	g_idle_add(discovery_state_changed, cb_data);
 }
 
 static bt_property_t *copy_properties(int num_properties,
-- 
1.8.5.2

[PATCH 03/13] android/tester: Execute device properties cbacks in main loop

[Jakub Tyszkowski]

Execute generic remote_device_properties_cb in tester's main loop.
---
 android/android-tester.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 0976081..35e375a 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -963,18 +963,36 @@ static void remote_setprop_device_properties_cb(bt_status_t status,
 	}
 }
 
-static void remote_device_properties_cb(bt_status_t status,
-				bt_bdaddr_t *bd_addr, int num_properties,
-				bt_property_t *properties)
+static gboolean remote_device_properties(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct bt_cb_data *cb_data = user_data;
 
 	if (data->test_init_done &&
-			test->expected_hal_cb.remote_device_properties_cb) {
-		test->expected_hal_cb.remote_device_properties_cb(status,
-					bd_addr, num_properties, properties);
-	}
+			test->expected_hal_cb.remote_device_properties_cb)
+		test->expected_hal_cb.remote_device_properties_cb(
+					cb_data->status, &cb_data->bdaddr,
+					cb_data->num, cb_data->props);
+
+	free_properties(cb_data->num, cb_data->props);
+	g_free(cb_data);
+
+	return FALSE;
+}
+
+static void remote_device_properties_cb(bt_status_t status,
+				bt_bdaddr_t *bd_addr, int num_properties,
+				bt_property_t *properties)
+{
+	struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+	cb_data->status = status;
+	cb_data->bdaddr = *bd_addr;
+	cb_data->num = num_properties;
+	cb_data->props = copy_properties(num_properties, properties);
+
+	g_idle_add(remote_device_properties, cb_data);
 }
 
 static bt_bdaddr_t enable_done_bdaddr_val = { {0x00} };
-- 
1.8.5.2

[PATCH 04/13] android/tester: Execute adapter props cbacks in main loop

[Jakub Tyszkowski]

Execute generic adapter_properties_cb in tester's main loop.
---
 android/android-tester.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 35e375a..3c4bf6c 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -915,19 +915,32 @@ static void check_count_properties_cb(bt_status_t status, int num_properties,
 		check_expected_property(properties[i]);
 }
 
+static gboolean adapter_properties(gpointer user_data)
+{
+	struct test_data *data = tester_get_data();
+	const struct generic_data *test = data->test_data;
+	struct bt_cb_data *cb_data = user_data;
+
+	if (data->test_init_done && test->expected_hal_cb.adapter_properties_cb)
+		test->expected_hal_cb.adapter_properties_cb(cb_data->status,
+						cb_data->num, cb_data->props);
+
+	free_properties(cb_data->num, cb_data->props);
+	g_free(cb_data);
+
+	return FALSE;
+}
 
 static void adapter_properties_cb(bt_status_t status, int num_properties,
 						bt_property_t *properties)
 {
-	struct test_data *data = tester_get_data();
-	const struct generic_data *test = data->test_data;
+	struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
 
-	if (data->test_init_done &&
-				test->expected_hal_cb.adapter_properties_cb) {
-		test->expected_hal_cb.adapter_properties_cb(
-							status, num_properties,
-							properties);
-	}
+	cb_data->status = status;
+	cb_data->num = num_properties;
+	cb_data->props = copy_properties(num_properties, properties);
+
+	g_idle_add(adapter_properties, cb_data);
 }
 
 static void remote_test_device_properties_cb(bt_status_t status,
-- 
1.8.5.2

[PATCH 05/13] android/tester: Execute adapter state changed cbacks in main loop

[Jakub Tyszkowski]

Execute generic adapter_state_changed_cb in tester's main loop.
---
 android/android-tester.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 3c4bf6c..a941e9f 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -599,19 +599,33 @@ static void disable_success_cb(bt_state_t state)
 	}
 }
 
-static void adapter_state_changed_cb(bt_state_t state)
+static gboolean adapter_state_changed(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct generic_data *test = data->test_data;
+	struct bt_cb_data *cb_data = user_data;
 
 	if (data->test_init_done &&
 			test->expected_hal_cb.adapter_state_changed_cb) {
-		test->expected_hal_cb.adapter_state_changed_cb(state);
-		return;
+		test->expected_hal_cb.adapter_state_changed_cb(cb_data->state);
+		goto cleanup;
 	}
 
-	if (!data->test_init_done && state == BT_STATE_ON)
+	if (!data->test_init_done && cb_data->state == BT_STATE_ON)
 		setup_powered_emulated_remote();
+
+cleanup:
+	g_free(cb_data);
+	return FALSE;
+}
+
+static void adapter_state_changed_cb(bt_state_t state)
+{
+	struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+	cb_data->state = state;
+
+	g_idle_add(adapter_state_changed, cb_data);
 }
 
 static void discovery_start_success_cb(bt_discovery_state_t state)
-- 
1.8.5.2

[PATCH 06/13] android/tester: Execute socket cbacks in main loop

[Jakub Tyszkowski]

Execute socket test's callbacks in tester's main loop.
---
 android/android-tester.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index a941e9f..c8496af 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -2778,9 +2778,11 @@ static void test_dev_setprop_disctimeout_fail(const void *test_data)
 }
 /* Test Socket HAL */
 
-static void adapter_socket_state_changed_cb(bt_state_t state)
+static gboolean adapter_socket_state_changed(gpointer user_data)
 {
-	switch (state) {
+	struct bt_cb_data *cb_data = user_data;
+
+	switch (cb_data->state) {
 	case BT_STATE_ON:
 		setup_powered_emulated_remote();
 		break;
@@ -2790,6 +2792,19 @@ static void adapter_socket_state_changed_cb(bt_state_t state)
 	default:
 		break;
 	}
+
+	g_free(cb_data);
+
+	return FALSE;
+}
+
+static void adapter_socket_state_changed_cb(bt_state_t state)
+{
+	struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
+
+	cb_data->state = state;
+
+	g_idle_add(adapter_socket_state_changed, cb_data);
 }
 
 const bt_bdaddr_t bdaddr_dummy = {
-- 
1.8.5.2

[PATCH 07/13] android/tester: Execute hh connection state cbacks in main loop

[Jakub Tyszkowski]

Execute generic HIDHost connection_state_cb in tester's main loop.
---
 android/android-tester.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index c8496af..d756dfc 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -127,6 +127,18 @@ struct bt_cb_data {
 	bt_property_t *props;
 };
 
+struct hh_cb_data {
+	bt_bdaddr_t bdaddr;
+
+	bthh_status_t status;
+	bthh_hid_info_t hid_info;
+	bthh_protocol_mode_t mode;
+	bthh_connection_state_t state;
+
+	uint8_t *report;
+	int size;
+};
+
 static char exec_dir[PATH_MAX + 1];
 
 static void mgmt_debug(const char *str, void *user_data)
@@ -3210,19 +3222,34 @@ clean:
 		close(sock_fd);
 }
 
-static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
-						bthh_connection_state_t state)
+static gboolean hidhost_connection_state(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct hidhost_generic_data *test = data->test_data;
+	struct hh_cb_data *cb_data = user_data;
 
 	data->cb_count++;
 
-	if (state == BTHH_CONN_STATE_CONNECTED)
+	if (cb_data->state == BTHH_CONN_STATE_CONNECTED)
 		tester_setup_complete();
 
 	if (test && test->expected_hal_cb.connection_state_cb)
-		test->expected_hal_cb.connection_state_cb(bd_addr, state);
+		test->expected_hal_cb.connection_state_cb(&cb_data->bdaddr,
+								cb_data->state);
+
+	g_free(cb_data);
+	return FALSE;
+}
+
+static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
+						bthh_connection_state_t state)
+{
+	struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+	cb_data->state = state;
+	cb_data->bdaddr = *bd_addr;
+
+	g_idle_add(hidhost_connection_state, cb_data);
 }
 
 static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
-- 
1.8.5.2

[PATCH 08/13] android/tester: Execute hh info cbacks in main loop

[Jakub Tyszkowski]

Execute generic HIDHost hid_info_cb in tester's main loop.
---
 android/android-tester.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index d756dfc..9cd0c78 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3263,15 +3263,30 @@ static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
 		test->expected_hal_cb.virtual_unplug_cb(bd_addr, status);
 }
 
-static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
+static gboolean hidhost_hid_info(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct hidhost_generic_data *test = data->test_data;
+	struct hh_cb_data *cb_data = user_data;
 
 	data->cb_count++;
 
 	if (test && test->expected_hal_cb.hid_info_cb)
-		test->expected_hal_cb.hid_info_cb(bd_addr, hid);
+		test->expected_hal_cb.hid_info_cb(&cb_data->bdaddr,
+							cb_data->hid_info);
+
+	g_free(cb_data);
+	return FALSE;
+}
+
+static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
+{
+	struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+	cb_data->bdaddr = *bd_addr;
+	cb_data->hid_info = hid;
+
+	g_idle_add(hidhost_hid_info, cb_data);
 }
 
 static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
-- 
1.8.5.2

[PATCH 09/13] android/tester: Execute hh protocol mode cbacks in main loop

[Jakub Tyszkowski]

Execute generic HIDHost protocol_mode_cb in tester's main loop.
---
 android/android-tester.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 9cd0c78..137f171 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3289,17 +3289,33 @@ static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
 	g_idle_add(hidhost_hid_info, cb_data);
 }
 
-static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
-						bthh_status_t status,
-						bthh_protocol_mode_t mode)
+static gboolean hidhost_protocol_mode(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct hidhost_generic_data *test = data->test_data;
+	struct hh_cb_data *cb_data = user_data;
 
 	data->cb_count++;
 
 	if (test && test->expected_hal_cb.protocol_mode_cb)
-		test->expected_hal_cb.protocol_mode_cb(bd_addr, status, mode);
+		test->expected_hal_cb.protocol_mode_cb(&cb_data->bdaddr,
+						cb_data->status, cb_data->mode);
+
+	g_free(cb_data);
+	return FALSE;
+}
+
+static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
+						bthh_status_t status,
+						bthh_protocol_mode_t mode)
+{
+	struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+	cb_data->bdaddr = *bd_addr;
+	cb_data->status = status;
+	cb_data->mode = mode;
+
+	g_idle_add(hidhost_protocol_mode, cb_data);
 }
 
 static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
-- 
1.8.5.2

[PATCH 10/13] android/tester: Execute hh report cbacks in main loop

[Jakub Tyszkowski]

Execute HIDHost generic get_report_cb in tester's main loop.
---
 android/android-tester.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 137f171..d9f9a57 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3318,17 +3318,34 @@ static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
 	g_idle_add(hidhost_protocol_mode, cb_data);
 }
 
-static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
-						uint8_t *report, int size)
+static gboolean hidhost_get_report(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct hidhost_generic_data *test = data->test_data;
+	struct hh_cb_data *cb_data = user_data;
 
 	data->cb_count++;
 
 	if (test && test->expected_hal_cb.get_report_cb)
-		test->expected_hal_cb.get_report_cb(bd_addr, status, report,
-									size);
+		test->expected_hal_cb.get_report_cb(&cb_data->bdaddr,
+			cb_data->status, cb_data->report, cb_data->size);
+
+	g_free(cb_data->report);
+	g_free(cb_data);
+	return FALSE;
+}
+
+static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
+						uint8_t *report, int size)
+{
+	struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+	cb_data->bdaddr = *bd_addr;
+	cb_data->status = status;
+	cb_data->report = g_memdup(report, size);
+	cb_data->size = size;
+
+	g_idle_add(hidhost_get_report, cb_data);
 }
 
 static bthh_callbacks_t bthh_callbacks = {
-- 
1.8.5.2

[PATCH 11/13] android/tester: Execute hh virtual unplug cbacks in main loop

[Jakub Tyszkowski]

Execute generic HIDHost virtual_unplug_cb in tester's main loop.
---
 android/android-tester.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index d9f9a57..36e3b7e 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -3252,15 +3252,30 @@ static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
 	g_idle_add(hidhost_connection_state, cb_data);
 }
 
-static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
+static gboolean hidhost_virual_unplug(gpointer user_data)
 {
 	struct test_data *data = tester_get_data();
 	const struct hidhost_generic_data *test = data->test_data;
+	struct hh_cb_data *cb_data = user_data;
 
 	data->cb_count++;
 
 	if (test && test->expected_hal_cb.virtual_unplug_cb)
-		test->expected_hal_cb.virtual_unplug_cb(bd_addr, status);
+		test->expected_hal_cb.virtual_unplug_cb(&cb_data->bdaddr,
+							cb_data->status);
+
+	g_free(cb_data);
+	return FALSE;
+}
+
+static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
+{
+	struct hh_cb_data *cb_data = g_new0(struct hh_cb_data, 1);
+
+	cb_data->bdaddr = *bd_addr;
+	cb_data->status = status;
+
+	g_idle_add(hidhost_virual_unplug, cb_data);
 }
 
 static gboolean hidhost_hid_info(gpointer user_data)
-- 
1.8.5.2

[PATCH 12/13] android/tester: Non-blocking check for daemon termination

[Jakub Tyszkowski]

This fixes the main loop being blocked, waiting for daemon termination
while daemon waits for freezed emulator to respond.
---
 android/android-tester.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 36e3b7e..921204d 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -2172,6 +2172,27 @@ static void setup_enabled_adapter(const void *test_data)
 		tester_setup_failed();
 }
 
+static gboolean check_daemon_term(gpointer user_data)
+{
+	int status;
+	struct test_data *data = tester_get_data();
+
+	if (!data)
+		return FALSE;
+
+	if ((waitpid(data->bluetoothd_pid, &status, WNOHANG))
+							!= data->bluetoothd_pid)
+		return TRUE;
+
+	if (WIFEXITED(status) && (WEXITSTATUS(status) == EXIT_SUCCESS)) {
+		tester_teardown_complete();
+		return FALSE;
+	}
+
+	tester_warn("Unexpected Daemon shutdown with status %d", status);
+	return FALSE;
+}
+
 static void teardown(const void *test_data)
 {
 	struct test_data *data = tester_get_data();
@@ -2191,10 +2212,7 @@ static void teardown(const void *test_data)
 
 	data->device->close(data->device);
 
-	if (data->bluetoothd_pid)
-		waitpid(data->bluetoothd_pid, NULL, 0);
-
-	tester_teardown_complete();
+	g_idle_add(check_daemon_term, NULL);
 }
 
 static void test_dummy(const void *test_data)
-- 
1.8.5.2

[PATCH 13/13] android/tester: Add guard for late callback calls

[Jakub Tyszkowski]

This fixes callbacks being called in wrong test case when current test
has already failed or timed out.
---
 android/android-tester.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/android/android-tester.c b/android/android-tester.c
index 921204d..f539b87 100644
--- a/android/android-tester.c
+++ b/android/android-tester.c
@@ -43,6 +43,8 @@
 
 #include "utils.h"
 
+gint scheduled_cbacks_num = 0;
+
 struct priority_property {
 	bt_property_t prop;
 	int prio;
@@ -628,6 +630,8 @@ static gboolean adapter_state_changed(gpointer user_data)
 
 cleanup:
 	g_free(cb_data);
+
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -637,6 +641,7 @@ static void adapter_state_changed_cb(bt_state_t state)
 
 	cb_data->state = state;
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(adapter_state_changed, cb_data);
 }
 
@@ -733,6 +738,7 @@ static gboolean discovery_state_changed(gpointer user_data)
 
 	g_free(cb_data);
 
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -741,6 +747,7 @@ static void discovery_state_changed_cb(bt_discovery_state_t state)
 	struct bt_cb_data *cb_data = g_new0(struct bt_cb_data, 1);
 
 	cb_data->state = state;
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(discovery_state_changed, cb_data);
 }
 
@@ -919,6 +926,7 @@ static gboolean device_found(gpointer user_data)
 	free_properties(cb_data->num, cb_data->props);
 	g_free(cb_data);
 
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -929,6 +937,7 @@ static void device_found_cb(int num_properties, bt_property_t *properties)
 	cb_data->num = num_properties;
 	cb_data->props = copy_properties(num_properties, properties);
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(device_found, cb_data);
 }
 
@@ -954,6 +963,7 @@ static gboolean adapter_properties(gpointer user_data)
 	free_properties(cb_data->num, cb_data->props);
 	g_free(cb_data);
 
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -966,6 +976,7 @@ static void adapter_properties_cb(bt_status_t status, int num_properties,
 	cb_data->num = num_properties;
 	cb_data->props = copy_properties(num_properties, properties);
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(adapter_properties, cb_data);
 }
 
@@ -1017,6 +1028,7 @@ static gboolean remote_device_properties(gpointer user_data)
 	free_properties(cb_data->num, cb_data->props);
 	g_free(cb_data);
 
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -1031,6 +1043,7 @@ static void remote_device_properties_cb(bt_status_t status,
 	cb_data->num = num_properties;
 	cb_data->props = copy_properties(num_properties, properties);
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(remote_device_properties, cb_data);
 }
 
@@ -2172,6 +2185,22 @@ static void setup_enabled_adapter(const void *test_data)
 		tester_setup_failed();
 }
 
+static gboolean check_callbacks_called(gpointer user_data)
+{
+	/*
+	 * Wait for all callbacks scheduled in current test context to execute
+	 * in main loop. This will avoid late callback calls after test case has
+	 * already failed or timed out.
+	 */
+
+	if (g_atomic_int_get(&scheduled_cbacks_num) == 0) {
+		tester_teardown_complete();
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static gboolean check_daemon_term(gpointer user_data)
 {
 	int status;
@@ -2185,7 +2214,7 @@ static gboolean check_daemon_term(gpointer user_data)
 		return TRUE;
 
 	if (WIFEXITED(status) && (WEXITSTATUS(status) == EXIT_SUCCESS)) {
-		tester_teardown_complete();
+		g_idle_add(check_callbacks_called, NULL);
 		return FALSE;
 	}
 
@@ -2825,6 +2854,7 @@ static gboolean adapter_socket_state_changed(gpointer user_data)
 
 	g_free(cb_data);
 
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -2834,6 +2864,7 @@ static void adapter_socket_state_changed_cb(bt_state_t state)
 
 	cb_data->state = state;
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(adapter_socket_state_changed, cb_data);
 }
 
@@ -3256,6 +3287,8 @@ static gboolean hidhost_connection_state(gpointer user_data)
 								cb_data->state);
 
 	g_free(cb_data);
+
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -3267,6 +3300,7 @@ static void hidhost_connection_state_cb(bt_bdaddr_t *bd_addr,
 	cb_data->state = state;
 	cb_data->bdaddr = *bd_addr;
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(hidhost_connection_state, cb_data);
 }
 
@@ -3283,6 +3317,8 @@ static gboolean hidhost_virual_unplug(gpointer user_data)
 							cb_data->status);
 
 	g_free(cb_data);
+
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -3293,6 +3329,7 @@ static void hidhost_virual_unplug_cb(bt_bdaddr_t *bd_addr, bthh_status_t status)
 	cb_data->bdaddr = *bd_addr;
 	cb_data->status = status;
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(hidhost_virual_unplug, cb_data);
 }
 
@@ -3309,6 +3346,8 @@ static gboolean hidhost_hid_info(gpointer user_data)
 							cb_data->hid_info);
 
 	g_free(cb_data);
+
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -3319,6 +3358,7 @@ static void hidhost_hid_info_cb(bt_bdaddr_t *bd_addr, bthh_hid_info_t hid)
 	cb_data->bdaddr = *bd_addr;
 	cb_data->hid_info = hid;
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(hidhost_hid_info, cb_data);
 }
 
@@ -3335,6 +3375,8 @@ static gboolean hidhost_protocol_mode(gpointer user_data)
 						cb_data->status, cb_data->mode);
 
 	g_free(cb_data);
+
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -3348,6 +3390,7 @@ static void hidhost_protocol_mode_cb(bt_bdaddr_t *bd_addr,
 	cb_data->status = status;
 	cb_data->mode = mode;
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(hidhost_protocol_mode, cb_data);
 }
 
@@ -3365,6 +3408,8 @@ static gboolean hidhost_get_report(gpointer user_data)
 
 	g_free(cb_data->report);
 	g_free(cb_data);
+
+	g_atomic_int_dec_and_test(&scheduled_cbacks_num);
 	return FALSE;
 }
 
@@ -3378,6 +3423,7 @@ static void hidhost_get_report_cb(bt_bdaddr_t *bd_addr, bthh_status_t status,
 	cb_data->report = g_memdup(report, size);
 	cb_data->size = size;
 
+	g_atomic_int_inc(&scheduled_cbacks_num);
 	g_idle_add(hidhost_get_report, cb_data);
 }
 
-- 
1.8.5.2

Re: [PATCH 00/13] Make android-tester callbacks thread safe

[Szymon Janc]

Hi Jakub,

On Friday 21 of February 2014 13:57:40 Jakub Tyszkowski wrote:
> 
> This patch set makes all callbacks executions in HAL's notification thread being
> transfered to tester's context. As test-specific callbacks are called by generic
> callbacks, making the later executed in tester's main loop makes all custom
> callbacks (and newly added) automatically executed in the right contex.
> 
> This is basically RFCv2 resend as PATCH with two follow-up patches.
> 
> Changes in relation to RFCv2:
>   * [PATCH 12/13]: Changed daemon termination checks to non-blocking
>   * [PATCH 13/13]: Added guard to execute all scheduled callbacks in current
>     test's context
> 
> Best regards,
> 
> Jakub Tyszkowski (13):
>   android/tester: Execute device found cbacks in main loop
>   android/tester: Execute discovery state cbacks in main loop
>   android/tester: Execute device properties cbacks in main loop
>   android/tester: Execute adapter props cbacks in main loop
>   android/tester: Execute adapter state changed cbacks in main loop
>   android/tester: Execute socket cbacks in main loop
>   android/tester: Execute hh connection state cbacks in main loop
>   android/tester: Execute hh info cbacks in main loop
>   android/tester: Execute hh protocol mode cbacks in main loop
>   android/tester: Execute hh report cbacks in main loop
>   android/tester: Execute hh virtual unplug cbacks in main loop
>   android/tester: Non-blocking check for daemon termination
>   android/tester: Add guard for late callback calls
> 
>  android/android-tester.c | 375 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 326 insertions(+), 49 deletions(-)

All patches except 12/13 are now applied. Thanks.
For waiting for daemon I pushed fix that is using signalfd instead.

-- 
Best regards, 
Szymon Janc