Make atomic variable to avoid race condition

- Issue:
Sometimes, the engine library can access invalid memory location when
the engine app tries to access callback.

- Solution:
Some engine API can access callback function in sttd_engine_agent.
However, these callbacks are not protected by multi threading. Thus,
thee engine app can access invalid pointer if engine library terminates
the engine internally and the app tries too access callback in sub
thread.
Therefore, this patch changes the global variable to atomic variable for
protecting those callback pointers and flags. And also this patch adds
null check for those pointers. Through this patch, engine library can
access those global variables in a safe way even if it is on multi thread
environment.

Change-Id: Ie501985b0ad5b1fdaf0937649cbacc37ab1e4b42
Signed-off-by: Suyeon Hwang <stom.hwang@samsung.com>

diff --git a/server/sttd_engine_agent.c b/server/sttd_engine_agent.c
index 3c4c543..790602d 100644 (file)
--- a/server/sttd_engine_agent.c
+++ b/server/sttd_engine_agent.c
@@ -14,6 +14,7 @@
 
 #include <dlfcn.h>
 #include <dirent.h>
+#include <stdatomic.h>
 
 #include "stt_defs.h"
 #include "stt_engine.h"
@@ -49,7 +50,7 @@ typedef struct _sttengine_info {
 } sttengine_info_s;
 
 /** stt engine agent init */
-static bool    g_agent_init;
+static atomic_bool g_agent_init;
 
 static sttengine_info_s* g_engine_info = NULL;
 
@@ -58,10 +59,10 @@ static char*        g_default_language = NULL;
 static bool    g_default_silence_detected;
 
 /** callback functions */
-static result_callback g_result_cb = NULL;
-static result_time_callback g_result_time_cb = NULL;
-static speech_status_callback g_speech_status_cb = NULL;
-static error_callback g_error_cb = NULL;
+static _Atomic result_callback g_result_cb = NULL;
+static _Atomic result_time_callback g_result_time_cb = NULL;
+static _Atomic speech_status_callback g_speech_status_cb = NULL;
+static _Atomic error_callback g_error_cb = NULL;
 
 /** callback functions */
 static bool __result_time_cb(int index, stte_result_time_event_e event, const char* text,
@@ -177,13 +178,13 @@ int sttd_engine_agent_release()
                __engine_agent_clear_engine(g_engine_info);
        }
 
+       g_agent_init = false;
+
        g_result_cb = NULL;
        g_speech_status_cb = NULL;
        g_error_cb = NULL;
        g_result_time_cb = NULL;
 
-       g_agent_init = false;
-
        return 0;
 }
 
@@ -378,10 +379,7 @@ int sttd_engine_agent_load_current_engine(stte_request_callback_s *callback)
 
 int sttd_engine_agent_unload_current_engine()
 {
-       if (false == g_agent_init) {
-               SLOG(LOG_ERROR, TAG_STTD, "[Engine Agent ERROR] Not Initialized ");
-               return STTD_ERROR_OPERATION_FAILED;
-       }
+       RETVM_IF(false == g_agent_init, STTD_ERROR_OPERATION_FAILED, "[Engine Agent ERROR] Not Initialized");
 
        /* Remove client */
        __engine_agent_check_engine_unload();
@@ -924,6 +922,8 @@ int sttd_engine_agent_send_result(stte_result_event_e event, const char* type, c
 
        SLOG(LOG_INFO, TAG_STTD, "[Server] ============================");
 
+       RETVM_IF(NULL == g_result_cb, STTD_ERROR_OPERATION_FAILED, "[Server ERROR] Callback is not set.");
+
        ret = g_result_cb(event, type, result, result_count, msg, user_data);
 
 #ifdef AUDIO_CREATE_ON_START
@@ -937,43 +937,34 @@ int sttd_engine_agent_send_result(stte_result_event_e event, const char* type, c
 
 int sttd_engine_agent_send_error(stte_error_e error, const char* msg)
 {
+       RETVM_IF(false == g_agent_init, STTD_ERROR_OPERATION_FAILED, "[Engine Agent ERROR] Not Initialized");
+
        /* check uid */
        unsigned int uid = stt_client_get_current_recognition();
 
-       char* err_msg = NULL;
-       int ret = STTD_ERROR_NONE;
-
-
-       if (NULL != msg) {
-               err_msg = strdup(msg);
-       }
-
-       ret = sttdc_send_error_signal(uid, error, err_msg);
+       int ret = sttdc_send_error_signal(uid, error, msg);
        if (0 != ret) {
-               SLOG(LOG_ERROR, TAG_STTD, "[Server ERROR] Fail to send error info.");
+               SLOG(LOG_ERROR, TAG_STTD, "[Server ERROR] Fail to send error info. (%d/%s)", ret, get_error_message(ret));
        }
 
-       if (NULL != err_msg) {
-               free(err_msg);
-               err_msg = NULL;
-       }
+       RETVM_IF(NULL == g_error_cb, STTD_ERROR_OPERATION_FAILED, "[Server ERROR] Callback is not set.");
 
-       ret = g_error_cb(error, msg);
-
-       return ret;
+       return g_error_cb(error, msg);
 }
 
 int sttd_engine_agent_send_speech_status(stte_speech_status_e status, void* user_data)
 {
-       int ret = STTD_ERROR_NONE;
        RETVM_IF(false == g_agent_init, STTD_ERROR_OPERATION_FAILED, "[Engine Agent ERROR] Not Initialized");
+       RETVM_IF(NULL == g_speech_status_cb, STTD_ERROR_OPERATION_FAILED, "[Server ERROR] Callback is not set.");
 
-       ret = g_speech_status_cb(status, user_data);
-       return ret;
+       return g_speech_status_cb(status, user_data);
 }
 
 static bool __result_time_cb(int index, stte_result_time_event_e event, const char* text, long start_time, long end_time, void* user_data)
 {
+       RETVM_IF(false == g_agent_init, false, "[Engine Agent ERROR] Not Initialized");
+       RETVM_IF(NULL == g_result_time_cb, false, "[Server ERROR] Callback is not set.");
+
        return g_result_time_cb(index, event, text, start_time, end_time, user_data);
 }
 

diff --git a/server/sttd_server.c b/server/sttd_server.c
index b7cdc6d..580b080 100644 (file)
--- a/server/sttd_server.c
+++ b/server/sttd_server.c
@@ -377,7 +377,7 @@ int __server_speech_status_callback(stte_speech_status_e status, void *user_para
 
 int __server_error_callback(stte_error_e error, const char* msg)
 {
-       SLOG(LOG_INFO, TAG_STTD, "[Server] Error Callback is called, error(%d), err_msg(%s)", error, msg);
+       SLOG(LOG_ERROR, TAG_STTD, "[Server] Error Callback is called, error(%d), err_msg(%s)", error, msg);
        ecore_main_loop_thread_safe_call_async(__cancel_by_error, NULL);
 
        return STTD_ERROR_NONE;