clientExample: refactor some code 92/276492/6
authorAdam Michalski <a.michalski2@partner.samsung.com>
Fri, 17 Jun 2022 09:25:50 +0000 (11:25 +0200)
committerMichal Bloch <m.bloch@samsung.com>
Wed, 22 Jun 2022 13:05:37 +0000 (15:05 +0200)
- removed race condition causing the main thread to terminate before
  returns from all callbacks occured
- refactored some code to conform to the Tizen coding standards
- removed some unused variables
- added some comments to the source code

Change-Id: Ifa080f9f48747db201b7e73016b414dd5fe1b400

clientExample/app/main.cpp

index 2c1ee30..8d9ff9f 100644 (file)
@@ -2,31 +2,40 @@
 #include <gio/gio.h>
 #include <thread>
 #include <cassert>
+#include <unistd.h>
 #include "sessiond.h"
 
-#define SESSION_UID 5001
-const int firstUser = 1; // 0 user is reserved
-const int lastUser = 11;
+#define SESSION_UID 5001    // default Tizen's 'owner' uid
+const int firstUser = 1;    // N.B. Starting from 1, as user number 0 is reserved
+const int lastUser = 10;
+const int noOfUsers = lastUser - firstUser + 1;
 
 GMutex mutex;
 volatile int callbackCount;
+GMainLoop *loop = nullptr;
 
 typedef struct {
        int callback_result;
 } test_user_data;
 
 typedef struct {
-    int session_uid;
-    int user_id;
-    uint64_t switch_id;
-    int callback_result;
-    int callback_reference;
-    int prev_user_id;
-    subsession_event_type_e event;
+       int session_uid;
+       int user_id;
+       uint64_t switch_id;
+       int callback_result;
+       int callback_reference;
+       int prev_user_id;
+       subsession_event_type_e event;
 } test_user_data_cb_t;
 
+// 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];
+
+int callback_pending_reference;
 
-void test_subsession_switch_user_completion_callback(subsession_event_info info, void *cb_data) {
+void test_subsession_switch_user_completion_callback(subsession_event_info info, void *cb_data)
+{
        g_atomic_int_inc(&callbackCount);
        test_user_data_cb_t *user_data = (test_user_data_cb_t *)cb_data;
        user_data->session_uid = info.session_uid;
@@ -36,32 +45,26 @@ void test_subsession_switch_user_completion_callback(subsession_event_info info,
        user_data->callback_result = 0;
 }
 
-typedef struct {
-       GMainLoop * loop;
-       test_user_data *user_data;
-}ud_ctrl;
-
-test_user_data userD[20];
-int callback_pending_reference;
-
-gboolean callback_pending(gpointer data) {
+gboolean callback_pending(gpointer data)
+{
+       GMainLoop *lp = (GMainLoop *)data;
 
-       ud_ctrl *ud = (ud_ctrl*)data;
        gboolean is_pending = g_main_context_pending(NULL);
        gint ctrl_value = g_atomic_int_get(&callback_pending_reference);
 
-       if(is_pending == TRUE) {
+       if (is_pending == TRUE)
                return TRUE;
-       }
 
-       if(ctrl_value >= 2) {
-               g_main_loop_quit((GMainLoop*)ud->loop);
-       }
+       // N.B. There are `noOfUsers` add and remove operations. Each add/remove
+       // increments the value of the `callback_pending_reference` variable.
+       if (ctrl_value >= noOfUsers * 2)
+               g_main_loop_quit(lp);
+
        return TRUE;
 }
 
-void test_reply_adduser_callback (int result, void *cb_data) {
-
+void test_reply_adduser_callback(int result, void *cb_data)
+{
        assert(cb_data);
 
        g_atomic_int_inc(&callback_pending_reference);
@@ -69,14 +72,11 @@ void test_reply_adduser_callback (int result, void *cb_data) {
        test_user_data *user_data = (test_user_data *)cb_data;
        user_data->callback_result = result;
 
-       if (result < 0)
-               return;
-
        g_mutex_unlock(&mutex);
 }
 
-void test_reply_removeuser_callback (int result, void *cb_data) {
-
+void test_reply_removeuser_callback(int result, void *cb_data)
+{
        assert(cb_data);
 
        g_atomic_int_inc(&callback_pending_reference);
@@ -85,12 +85,12 @@ void test_reply_removeuser_callback (int result, void *cb_data) {
        user_data->callback_result = result;
 }
 
-void test_reply_switchuser_callback (int result, void *cb_data){
+void test_reply_switchuser_callback(int result, void *cb_data)
+{
        test_user_data *user_data = (test_user_data *)cb_data;
        user_data->callback_result = result;
        g_mutex_unlock(&mutex);
 }
-GMainLoop* loop = NULL;
 
 int switch_user_test(int user_id)
 {
@@ -99,10 +99,11 @@ int switch_user_test(int user_id)
        test_switch_ud.callback_result = -1;
 
        g_mutex_trylock(&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)
+       int switch_user_res = subsession_switch_user(SESSION_UID, user_id,
+               test_reply_switchuser_callback, (void *)&test_switch_ud);
+       if (switch_user_res != 0)
        {
-               printf("Error subsession_switch_user res is%d\n",switch_user_res);
+               printf("Error subsession_switch_user res is%d\n", switch_user_res);
                return -1;
        }
        g_mutex_lock(&mutex);
@@ -111,26 +112,26 @@ int switch_user_test(int user_id)
        return new_user;
 }
 
-
-int main(int argc,char *argv[]) {
-
-       loop = g_main_loop_new (NULL, FALSE);
+int main(int argc, char *argv[])
+{
+       loop = g_main_loop_new(NULL, FALSE);
        g_mutex_init(&mutex);
-       std::thread t1(g_main_loop_run, loop);
+       std::thread g_main_loop_thread(g_main_loop_run, loop);
        g_atomic_int_set(&callback_pending_reference, 0);
 
        ///===================================///
        printf("Test program start\nCreating test users...");
 
-       for (int i = firstUser; i < lastUser; ++i)
+       for (int i = firstUser; i <= lastUser; ++i)
        {
                g_mutex_trylock(&mutex);
                userD[i].callback_result = i * 10;
-               int add_user_res = subsession_add_user(SESSION_UID, i, test_reply_adduser_callback, (void *)&userD[i]);
+               int add_user_res = subsession_add_user(SESSION_UID, i,
+                       test_reply_adduser_callback, (void *)&userD[i]);
 
-               ifadd_user_res != 0)
+               if (add_user_res != 0)
                {
-                       printf("Error subsession_add_user res is %d\n", add_user_res);
+                       printf("Error: subsession_add_user result is %d\n", add_user_res);
                        return -1;
                }
                g_mutex_lock(&mutex);
@@ -141,39 +142,35 @@ int main(int argc,char *argv[]) {
        int *userlist;
        ///===================================///
        subsession_get_user_list(SESSION_UID, (int **)&userlist, &registered_users);
-       printf("No of registered users [%d/%d]...", registered_users, lastUser - firstUser);
-       if(lastUser - firstUser == registered_users)
-       {
+       printf("No of registered users [%d/%d]...", registered_users, noOfUsers);
+       if (noOfUsers == registered_users)
                printf("\033[0;32mok\033[0;37m\n");
-       }
        else
        {
                printf("Register event callback error\n");
                return -1;
        }
 
-       if(registered_users != lastUser - firstUser)
-               return 1;
-
        printf("Lp. ");
-       for(int i = 0; i< registered_users; ++i)
+       for (int i = 0; i < registered_users; ++i)
                printf(" %2d ", i);
        printf("\nId. ");
-       for(int i = 0; i< registered_users; ++i)
+       for (int i = 0; i < registered_users; ++i)
                printf(" %2d ", userlist[i]);
        printf("\n");
        ///===================================///
        printf("Switching users test...");
 
-       callbackCount =0;
+       callbackCount = 0;
 
        test_user_data_cb_t data;
-       subsession_register_event_callback(SESSION_UID, SUBSESSION_SWITCH_USER_COMPLETION, test_subsession_switch_user_completion_callback, (void *)&data);
+       subsession_register_event_callback(SESSION_UID, SUBSESSION_SWITCH_USER_COMPLETION,
+               test_subsession_switch_user_completion_callback, (void *)&data);
 
-       for(int i = firstUser; i< registered_users + firstUser; ++i)
+       for (int i = firstUser; i < registered_users + firstUser; ++i)
        {
                int res = switch_user_test(i);
-               if(res != i)
+               if (res != i)
                {
                        printf("Switch user error\n");
                        return -1;
@@ -182,11 +179,10 @@ int main(int argc,char *argv[]) {
 
        printf("\033[0;32mdone\033[0;37m\n");
 
-       printf("Register event callback test [%d/%d]...", g_atomic_int_get(&callbackCount), registered_users );
-       if(g_atomic_int_get(&callbackCount) == registered_users)
-       {
+       printf("Register event callback test [%d/%d]...",
+               g_atomic_int_get(&callbackCount), registered_users);
+       if (g_atomic_int_get(&callbackCount) == registered_users)
                printf("\033[0;32mok\033[0;37m\n");
-       }
        else
        {
                printf("Register event callback error\n");
@@ -195,22 +191,20 @@ int main(int argc,char *argv[]) {
        ///===================================///
        printf("Subsession unregister event callback test...");
 
-       callbackCount =0;
+       callbackCount = 0;
        subsession_unregister_event_callback(SESSION_UID, SUBSESSION_SWITCH_USER_COMPLETION);
-       for(int i = firstUser; i< registered_users + firstUser; ++i)
+       for (int i = firstUser; i < registered_users + firstUser; ++i)
        {
                int res = switch_user_test(i);
-               if(res != i)
+               if (res != i)
                {
                        printf("Switch user error\n");
                        return -1;
                }
 
        }
-       if(g_atomic_int_get(&callbackCount) == 0)
-       {
+       if (g_atomic_int_get(&callbackCount) == 0)
                printf("\033[0;32mdone\033[0;37m\n");
-       }
        else
        {
                printf("Register event callback error\n");
@@ -219,17 +213,18 @@ int main(int argc,char *argv[]) {
        ///======================================///
        printf("Removing users...");
        int end_res = switch_user_test(0);
-       if(end_res != 0)
+       if (end_res != 0)
        {
                printf("Error setting user to 0\n");
                return -1;
        }
-       for (int i = firstUser; i < lastUser; ++i)
+       for (int i = firstUser; i <= lastUser; ++i)
        {
                test_user_data test_remove_ud;
                test_remove_ud.callback_result = -1;
-               int remove_user_res = subsession_remove_user(SESSION_UID, i, test_reply_removeuser_callback, (void *)&test_remove_ud);
-               if(remove_user_res != 0)
+               int remove_user_res = subsession_remove_user(SESSION_UID, i,
+                       test_reply_removeuser_callback, (void *)&test_remove_ud);
+               if (remove_user_res != 0)
                {
                        printf("removing user %d failed code: %d\n", i,remove_user_res);
                        return -1;
@@ -237,12 +232,16 @@ int main(int argc,char *argv[]) {
        }
        printf("\033[0;32mdone\033[0;37m\n");
 
-       ud_ctrl ud;
-       ud.loop = loop;
+       // Create a new idle source, add a message idle function that ultimately terminates
+       // the message loop, making sure in advance that all callbacks have finished their actions.
+       GSource *idle_source = g_idle_source_new();
+       g_source_set_callback(idle_source, (GSourceFunc)callback_pending, loop, NULL);
+       g_source_attach(idle_source, NULL);
+       g_source_unref(idle_source);
 
-       g_idle_add(callback_pending,(gpointer*)&ud);
+       // Wait for the message loop thread
+       g_main_loop_thread.join();
 
-       t1.join();
        g_mutex_unlock(&mutex);
        g_mutex_clear(&mutex);
        printf("Test program end\n");