Client example: rework mutexes

 * add a lot of commentary, including to the newly added
   example of passing values to the callback.

 * use a conditional variable instead of a single mutex.
   This is because glib mutexes (and their underlying
   default POSIX mutexes) have undefined behaviour when
   trying to lock an already owned mutex from the owning
   thread. As a side effect, this gets rid of the trylock
   mechanism which (even though it worked) was confusing.

 * fix the mutex being left locked on subsession call error.

Change-Id: I1afa973bcae57ff8c7434d2365d51893ff7696d0
Signed-off-by: Michal Bloch <m.bloch@samsung.com>

diff --git a/clientExample/app/main.cpp b/clientExample/app/main.cpp
index 537513d..aef1082 100644 (file)
--- a/clientExample/app/main.cpp
+++ b/clientExample/app/main.cpp
@@ -11,7 +11,10 @@ static constexpr int firstUser = 1;  // N.B. Starting from 1, as user number 0 i
 static constexpr int lastUser = 10;
 static constexpr int noOfUsers = lastUser - firstUser + 1;
 
+static constexpr int CALLBACK_DATA_MAGIC = 0xDEAD50UL; // arbitrary; different from any SUBSESSION_ERROR calls
+
 GMutex mutex;
+GCond cond;
 volatile int callbackCount;
 
 typedef struct {
@@ -30,7 +33,7 @@ typedef struct {
 
 // N.B. There are `noOfUsers` users added, but we're treating
 // the array as 1-based, so we need to add 1 more element.
-test_user_data userD[noOfUsers + 1];
+volatile test_user_data userD[noOfUsers + 1];
 
 int callback_pending_reference = 0;
 
@@ -74,11 +77,21 @@ void test_reply_adduser_callback(int result, void *cb_data)
 {
        assert(cb_data);
 
+       /* Use a conditional variable (with the associated mutex)
+        * to make calls synchronous. This is just for example purposes,
+        * it is not required. */
+
+       g_mutex_lock(&mutex);
        g_atomic_int_inc(&callback_pending_reference);
 
        test_user_data *user_data = (test_user_data *)cb_data;
+
+       /* We set the value below ourselves. Arbitrary metadata
+        * can be passed to and from the callback. */
+       assert(user_data->callback_result == CALLBACK_DATA_MAGIC);
        user_data->callback_result = result;
 
+       g_cond_signal(&cond);
        g_mutex_unlock(&mutex);
 }
 
@@ -86,6 +99,10 @@ void test_reply_removeuser_callback(int result, void *cb_data)
 {
        assert(cb_data);
 
+       /* NB: no mutex or waiting here. The callback is ran
+        * asynchronously via the main loop. See below,
+        * especially `wait_for_all_pending_callbacks`. */
+
        g_atomic_int_inc(&callback_pending_reference);
 
        test_user_data *user_data = (test_user_data *)cb_data;
@@ -94,25 +111,39 @@ void test_reply_removeuser_callback(int result, void *cb_data)
 
 void test_reply_switchuser_callback(int result, void *cb_data)
 {
+       g_mutex_lock(&mutex);
+
        test_user_data *user_data = (test_user_data *)cb_data;
        user_data->callback_result = result;
+
+       g_cond_signal(&cond);
        g_mutex_unlock(&mutex);
 }
 
 int switch_user_test(int user_id)
 {
-       test_user_data test_switch_ud;
-       test_switch_ud.callback_result = -1;
+       volatile test_user_data test_switch_ud;
+       test_switch_ud.callback_result = CALLBACK_DATA_MAGIC;
 
-       g_mutex_trylock(&mutex);
+       g_mutex_lock(&mutex);
        int switch_user_res = subsession_switch_user(SESSION_UID, user_id,
                test_reply_switchuser_callback, (void *)&test_switch_ud);
        if (switch_user_res != 0)
        {
+               g_mutex_unlock(&mutex);
                printf("Error subsession_switch_user res is%d\n", switch_user_res);
                return -1;
        }
-       g_mutex_lock(&mutex);
+
+       /* The wait has to be in a loop because of
+        * spurious wakeups, this is a glib issue.
+        *
+        * Of course you can use any synchronisation
+        * mechanism you want, including none at all. */
+       while (test_switch_ud.callback_result == CALLBACK_DATA_MAGIC)
+               g_cond_wait(&cond, &mutex);
+
+       g_mutex_unlock(&mutex);
 
        int new_user;
        int r = subsession_get_current_user(SESSION_UID, &new_user);
@@ -139,6 +170,7 @@ void wait_for_all_pending_callbacks(GMainLoop *loop, GThread *loop_thread)
 
 int main(int argc, char *argv[])
 {
+       g_cond_init(&cond);
        g_mutex_init(&mutex);
 
        GMainLoop *loop = g_main_loop_new(NULL, FALSE);
@@ -149,17 +181,23 @@ int main(int argc, char *argv[])
 
        for (int i = firstUser; i <= lastUser; ++i)
        {
-               g_mutex_trylock(&mutex);
-               userD[i].callback_result = i * 10;
+               g_mutex_lock(&mutex);
+               userD[i].callback_result = CALLBACK_DATA_MAGIC;
                int add_user_res = subsession_add_user(SESSION_UID, i,
                        test_reply_adduser_callback, (void *)&userD[i]);
 
                if (add_user_res != SUBSESSION_ERROR_NONE)
                {
+                       g_mutex_unlock(&mutex);
                        printf("Error: subsession_add_user result is %d\n", add_user_res);
                        return EXIT_FAILURE;
                }
-               g_mutex_lock(&mutex);
+
+               // see `switch_user_test()`
+               while (userD[i].callback_result == CALLBACK_DATA_MAGIC)
+                       g_cond_wait(&cond, &mutex);
+
+               g_mutex_unlock(&mutex);
        }
        green_print("done");
 
@@ -276,8 +314,9 @@ int main(int argc, char *argv[])
 
        wait_for_all_pending_callbacks(loop, loop_thread);
 
-       g_mutex_unlock(&mutex);
        g_mutex_clear(&mutex);
+       g_cond_clear(&cond);
+
        printf("Test program end\n");
        return EXIT_SUCCESS;
 }