Wait to start async client subthread until api returned

For robust calling registered callbacks, wait to start async client
subthread until api returned, controlled by handle-ext mutex lock.
Some client gets undefined behavior when registered callback called
earlier than returning async API...

Change-Id: I31ac4596cb43d2b67b9fe8095ed2a4c98779e17e
Signed-off-by: Kyungwook Tak <k.tak@samsung.com>

diff --git a/src/framework/client/content-screening.cpp b/src/framework/client/content-screening.cpp
index a9fbabc..3da1843 100644 (file)
--- a/src/framework/client/content-screening.cpp
+++ b/src/framework/client/content-screening.cpp
@@ -413,6 +413,8 @@ int csr_cs_scan_files_async(csr_cs_context_h handle, const char *file_paths[],
                EXCEPTION_SAFE_END
        });
 
+       std::lock_guard<std::mutex> l(hExt->m_dispatchMutex);
+
        hExt->dispatchAsync(task);
 
        return CSR_ERROR_NONE;
@@ -450,6 +452,8 @@ int csr_cs_scan_dir_async(csr_cs_context_h handle, const char *dir_path,
                EXCEPTION_SAFE_END
        });
 
+       std::lock_guard<std::mutex> l(hExt->m_dispatchMutex);
+
        hExt->dispatchAsync(task);
 
        return CSR_ERROR_NONE;
@@ -513,6 +517,8 @@ int csr_cs_scan_dirs_async(csr_cs_context_h handle, const char *dir_paths[],
                EXCEPTION_SAFE_END
        });
 
+       std::lock_guard<std::mutex> l(hExt->m_dispatchMutex);
+
        hExt->dispatchAsync(task);
 
        return CSR_ERROR_NONE;

diff --git a/src/framework/client/handle-ext.cpp b/src/framework/client/handle-ext.cpp
index 9d1aea1..0b89eee 100644 (file)
--- a/src/framework/client/handle-ext.cpp
+++ b/src/framework/client/handle-ext.cpp
@@ -82,7 +82,12 @@ void HandleExt::dispatchAsync(const std::shared_ptr<Task> &f)
        this->m_worker = std::thread([this, f] {
                DEBUG("client async thread dispatched! tid: " << std::this_thread::get_id());
 
-               (*f)();
+               {
+                       // Wait for client lib API func returned & mutex freed by scoped-lock dtor
+                       // This is for invoking registered callbacks follows returning API func
+                       std::lock_guard<std::mutex> _l(this->m_dispatchMutex);
+               }
+                       (*f)();
 
                {
                        std::lock_guard<std::mutex> _l(this->m_flagMutex);

diff --git a/src/framework/client/handle-ext.h b/src/framework/client/handle-ext.h
index 756d9b1..e885d7e 100644 (file)
--- a/src/framework/client/handle-ext.h
+++ b/src/framework/client/handle-ext.h
@@ -42,6 +42,7 @@ public:
        bool isRunning(void) const;
 
        Callback m_cb; // TODO: to refine..
+       std::mutex m_dispatchMutex;
 
        virtual void add(ResultPtr &&) override;
        virtual void add(ResultListPtr &&) override;

diff --git a/test/test-api-content-screening-async.cpp b/test/test-api-content-screening-async.cpp
index 8261c20..96b70b9 100644 (file)
--- a/test/test-api-content-screening-async.cpp
+++ b/test/test-api-content-screening-async.cpp
@@ -60,20 +60,26 @@ struct AsyncTestContext {
        std::vector<std::string> scannedList;
        std::vector<csr_cs_malware_h> detectedList;
        int errorCode;
+       bool apiReturned;
 
-       AsyncTestContext() :
+       AsyncTestContext(bool raceTest = false) :
                scannedCnt(0),
                detectedCnt(0),
                completedCnt(0),
                cancelledCnt(0),
-               errorCnt(0) {}
+               errorCnt(0),
+               apiReturned(!raceTest) {}
 };
 
 void on_scanned(const char *file, void *userdata)
 {
-       BOOST_MESSAGE("on_scanned. file[" << file << "] scanned!");
        auto ctx = reinterpret_cast<AsyncTestContext *>(userdata);
 
+       BOOST_REQUIRE_MESSAGE(ctx->apiReturned,
+               "API not returned yet but scanned callback called on file: " << file);
+
+       BOOST_MESSAGE("on_scanned. file[" << file << "] scanned!");
+
        std::lock_guard<std::mutex> l(ctx->m_vec);
 
        ctx->scannedCnt++;
@@ -82,10 +88,14 @@ void on_scanned(const char *file, void *userdata)
 
 void on_detected(csr_cs_malware_h detected, void *userdata)
 {
+       auto ctx = reinterpret_cast<AsyncTestContext *>(userdata);
+
+       BOOST_REQUIRE_MESSAGE(ctx->apiReturned,
+               "API not returned yet but detected callback called");
+
        Test::ScopedCstr file_name;
        ASSERT_SUCCESS(csr_cs_malware_get_file_name(detected, &file_name.ptr));
        BOOST_MESSAGE("on_detected. file[" << file_name.ptr << "] detected!");
-       auto ctx = reinterpret_cast<AsyncTestContext *>(userdata);
 
        std::lock_guard<std::mutex> l(ctx->m_vec);
 
@@ -95,9 +105,14 @@ void on_detected(csr_cs_malware_h detected, void *userdata)
 
 void on_error(int ec, void *userdata)
 {
+       auto ctx = reinterpret_cast<AsyncTestContext *>(userdata);
+
+       BOOST_REQUIRE_MESSAGE(ctx->apiReturned,
+               "API not returned yet but error callback called with error: " <<
+               Test::capi_ec_to_string(static_cast<csr_error_e>(ec)));
+
        BOOST_MESSAGE("on_error. async request done with error: " <<
                                  Test::capi_ec_to_string(static_cast<csr_error_e>(ec)));
-       auto ctx = reinterpret_cast<AsyncTestContext *>(userdata);
 
        ctx->errorCnt++;
        ctx->errorCode = ec;
@@ -106,16 +121,24 @@ void on_error(int ec, void *userdata)
 
 void on_completed(void *userdata)
 {
-       BOOST_MESSAGE("on_completed. async request completed succesfully.");
        auto ctx = reinterpret_cast<AsyncTestContext *>(userdata);
+
+       BOOST_REQUIRE_MESSAGE(ctx->apiReturned,
+               "API not returned yet but completed callback called");
+
+       BOOST_MESSAGE("on_completed. async request completed succesfully.");
        ctx->completedCnt++;
        ctx->cv.notify_one();
 }
 
 void on_cancelled(void *userdata)
 {
-       BOOST_MESSAGE("on_cancelled. async request canceled!");
        auto ctx = reinterpret_cast<AsyncTestContext *>(userdata);
+
+       BOOST_REQUIRE_MESSAGE(ctx->apiReturned,
+               "API not returned yet but cancelled callback called");
+
+       BOOST_MESSAGE("on_cancelled. async request canceled!");
        ctx->cancelledCnt++;
        ctx->cv.notify_one();
 }
@@ -910,4 +933,32 @@ BOOST_AUTO_TEST_CASE(get_malware_after_async)
        EXCEPTION_GUARD_END
 }
 
+BOOST_AUTO_TEST_CASE(async_api_returning_and_callback_race)
+{
+       EXCEPTION_GUARD_START
+
+       auto c = Test::Context<csr_cs_context_h>();
+       auto context = c.get();
+
+       install_test_files();
+
+       set_default_callback(context);
+
+       const char *files[2] = {
+               TEST_FILE_NORMAL,
+               TEST_FILE_HIGH
+       };
+
+       AsyncTestContext testCtx(true);
+
+       ASSERT_SUCCESS(csr_cs_scan_files_async(context, files, 2, &testCtx));
+       testCtx.apiReturned = true;
+
+       std::unique_lock<std::mutex> l(testCtx.m);
+       testCtx.cv.wait(l);
+       l.unlock();
+
+       EXCEPTION_GUARD_END
+}
+
 BOOST_AUTO_TEST_SUITE_END()