From: Youngjae Shin Date: Tue, 26 Nov 2019 04:03:41 +0000 (+0900) Subject: revise the logic of handling async action X-Git-Tag: submit/tizen/20200319.043412~29 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b89b992dfa352a282ee2b59fa68a2b2e2f37ff37;p=platform%2Fcore%2Fsystem%2Fmodes.git revise the logic of handling async action --- diff --git a/example/mode/tizen_applyNormalAsync1_mode.xml b/example/mode/tizen_applyNormalAsync1_mode.xml deleted file mode 100644 index 669b73b..0000000 --- a/example/mode/tizen_applyNormalAsync1_mode.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - 5 - 5 - 5 - 5 - 5 - - diff --git a/example/mode/tizen_applyNormalSync1_mode.xml b/example/mode/tizen_applyNormalSync1_mode.xml deleted file mode 100644 index 2fbf7fe..0000000 --- a/example/mode/tizen_applyNormalSync1_mode.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - 5 - 5 - 5 - 5 - 5 - - diff --git a/example/mode/tizen_applyOneshotAsync1_mode.xml b/example/mode/tizen_applyOneshotAsync1_mode.xml deleted file mode 100644 index 747104f..0000000 --- a/example/mode/tizen_applyOneshotAsync1_mode.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - 5 - 5 - 5 - 5 - 5 - - diff --git a/example/mode/tizen_applyOneshotSync1_mode.xml b/example/mode/tizen_applyOneshotSync1_mode.xml deleted file mode 100644 index 41e902d..0000000 --- a/example/mode/tizen_applyOneshotSync1_mode.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - 5 - 5 - 5 - 5 - 5 - - diff --git a/example/mode/tizen_asyncEx1_mode.xml b/example/mode/tizen_asyncEx1_mode.xml new file mode 100644 index 0000000..06dac5a --- /dev/null +++ b/example/mode/tizen_asyncEx1_mode.xml @@ -0,0 +1,10 @@ + + + + 5 + 5 + 5 + 5 + 5 + + diff --git a/example/mode/tizen_asyncEx2_mode.xml b/example/mode/tizen_asyncEx2_mode.xml new file mode 100644 index 0000000..8310a2a --- /dev/null +++ b/example/mode/tizen_asyncEx2_mode.xml @@ -0,0 +1,10 @@ + + + + 5 + 5 + 5 + 5 + 5 + + diff --git a/example/mode/tizen_asyncFail1_mode.xml b/example/mode/tizen_asyncFail1_mode.xml new file mode 100644 index 0000000..b764ffa --- /dev/null +++ b/example/mode/tizen_asyncFail1_mode.xml @@ -0,0 +1,10 @@ + + + + 5 + 5 + 5 + 5 + 1 + + diff --git a/example/mode/tizen_asyncFail2_mode.xml b/example/mode/tizen_asyncFail2_mode.xml new file mode 100644 index 0000000..74c4263 --- /dev/null +++ b/example/mode/tizen_asyncFail2_mode.xml @@ -0,0 +1,10 @@ + + + + 5 + 5 + 5 + 5 + 5 + + diff --git a/packaging/modes.spec b/packaging/modes.spec index ea8d0e5..6dec3e4 100644 --- a/packaging/modes.spec +++ b/packaging/modes.spec @@ -100,7 +100,7 @@ install -d -m 755 %{buildroot}%{modes_ro_dir}/rule install -d -m 755 %{buildroot}%{modes_rw_dir}/custom-mode install -d -m 755 %{buildroot}%{modes_rw_dir}/undo-info install -m 0644 example/mode/*ex*_mode.xml %{buildroot}%{modes_ro_dir}/mode/ -install -m 0644 example/mode/*apply*_mode.xml %{buildroot}%{modes_ro_dir}/mode/ +install -m 0644 example/mode/*sync*_mode.xml %{buildroot}%{modes_ro_dir}/mode/ install -m 0644 example/rule/*ex*_rule.xml %{buildroot}%{modes_ro_dir}/rule/ install -m 0644 example/mode/*conflict*_mode.xml %{buildroot}%{modes_test_dir}/ install -m 0644 example/mode/*invalid*_mode.xml %{buildroot}%{modes_test_dir}/ diff --git a/plugin/TestPlugin.cpp b/plugin/TestPlugin.cpp index b6d1fa4..744311b 100644 --- a/plugin/TestPlugin.cpp +++ b/plugin/TestPlugin.cpp @@ -67,23 +67,25 @@ TestPlugin::~TestPlugin() int TestPlugin::set(const std::string &key, int val, PluginAction **piAction) { - DBG("TestPlugin::set int ( %s, %d )", key.c_str(), val); + DBG("set(%s, %d) Begin", key.c_str(), val); - if (piAction) { - TestPluginAction *testAction = new TestPluginAction(); - *piAction = testAction; - } + TestPluginAction *testAction = new TestPluginAction(); - if (key.compare("sleep") == 0) { - DBG("Sleep %d seconds", val); + if ("sleep" == key) { std::this_thread::sleep_for(std::chrono::seconds(val)); - DBG("TestPlugin::set int ( %s, %d ) Wakeup", key.c_str(), val); + DBG("set(%s, %d) End", key.c_str(), val); } else if (key.compare("sleepErrorReturn") == 0) { - DBG("Sleep %d seconds", val); std::this_thread::sleep_for(std::chrono::seconds(val)); - DBG("TestPlugin::set int ( %s, %d ) Wakeup", key.c_str(), val); + delete testAction; + DBG("set(%s, %d) End", key.c_str(), val); return MODES_ERROR_SYSTEM; } + + if (piAction) + *piAction = testAction; + else + delete testAction; + return MODES_ERROR_NONE; } diff --git a/supervisor/Action.cpp b/supervisor/Action.cpp index 2aa3dc8..d2bb431 100644 --- a/supervisor/Action.cpp +++ b/supervisor/Action.cpp @@ -21,13 +21,13 @@ MODES_NAMESPACE_USE; Action::Action() - : isChanged(false), plugin(nullptr), piAction(nullptr), stopOnErr(false), type(SYNC), restriction(REQ_NONE) + : isChanged(false), type(SYNC), plugin(nullptr), piAction(nullptr), stopOnErr(false), restriction(REQ_NONE) { } Action::Action(const std::string &name) - : ruleName(name), isChanged(false), plugin(nullptr), piAction(nullptr), stopOnErr(false), type(SYNC), restriction(REQ_NONE) + : ruleName(name), isChanged(false), type(SYNC), plugin(nullptr), piAction(nullptr), stopOnErr(false), restriction(REQ_NONE) { } diff --git a/supervisor/Action.h b/supervisor/Action.h index ffda280..ae99734 100644 --- a/supervisor/Action.h +++ b/supervisor/Action.h @@ -61,12 +61,12 @@ protected: static void valueChangedCallback(void *userData); std::string ruleName; bool isChanged; + ActionType type; Plugin *plugin; PluginAction *piAction; private: std::string id; bool stopOnErr; - ActionType type; ActionRestrict restriction; }; diff --git a/supervisor/Mode.cpp b/supervisor/Mode.cpp index 41ef6fb..62e4bdc 100644 --- a/supervisor/Mode.cpp +++ b/supervisor/Mode.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ #include "Mode.h" + +#include #include "modes_constants.h" #include "ModesEx.h" @@ -83,10 +85,14 @@ int Mode::apply() { std::list>::iterator it; for (it = actionList.begin(); it != actionList.end(); it++) { - int ret = (*it)->apply(); - if (MODES_ERROR_NONE != ret && (*it)->getStopOnErr()) { - ERR("Action(%s) apply Fail(%d)", (*it)->getRuleName().c_str(), ret); - return ret; + if (Action::ActionType::ASYNC == (*it)->getType()) { + std::thread(&Action::apply, *it).detach(); + } else { + int ret = (*it)->apply(); + if (MODES_ERROR_NONE != ret && (*it)->getStopOnErr()) { + ERR("Action(%s) apply Fail(%d)", (*it)->getRuleName().c_str(), ret); + return ret; + } } } return MODES_ERROR_NONE; @@ -96,10 +102,14 @@ int Mode::applyOneShot() { std::list>::iterator it; for (it = actionList.begin(); it != actionList.end(); it++) { - int ret = (*it)->applyOneShot(); - if (MODES_ERROR_NONE != ret && (*it)->getStopOnErr()) { - ERR("Action(%s) applyOneShot Fail(%d)", (*it)->getRuleName().c_str(), ret); - return ret; + if (Action::ActionType::ASYNC == (*it)->getType()) { + std::thread(&Action::applyOneShot, *it).detach(); + } else { + int ret = (*it)->applyOneShot(); + if (MODES_ERROR_NONE != ret && (*it)->getStopOnErr()) { + ERR("Action(%s) applyOneShot Fail(%d)", (*it)->getRuleName().c_str(), ret); + return ret; + } } } return MODES_ERROR_NONE; diff --git a/supervisor/Mode.h b/supervisor/Mode.h index 2f3fc51..0a92e6e 100644 --- a/supervisor/Mode.h +++ b/supervisor/Mode.h @@ -46,8 +46,6 @@ public: void addUndo(Action *action); std::list> getUndoList() const; - void addUndoInfo(const std::string &rule, const std::string &info); - int apply(); int applyOneShot(); void undo(); diff --git a/supervisor/TAction.h b/supervisor/TAction.h index 030474b..7cb0533 100644 --- a/supervisor/TAction.h +++ b/supervisor/TAction.h @@ -29,10 +29,19 @@ template class TAction : public Action { public: TAction(const std::string &name) - : Action(name) + : Action(name), undoing(false) { } + ~TAction() + { + if (undoing) + undo(); + + if (piAction) + piAction->unSetChangedCallback(valueChangedCallback, this); + } + void setValueAliases(const std::map &list) { valueAliases.clear(); @@ -101,17 +110,46 @@ public: int apply() override { - return applyImpl(false, getType() == Action::ActionType::ASYNC); + RETVM_IF(NULL == plugin, MODES_ERROR_NO_DATA, "Action(%s) : No plugin", ruleName.c_str()); + + int pos = ruleName.find_first_of("."); + PluginAction *tmpAction = nullptr; + int ret = plugin->set(ruleName.substr(pos + 1), value, &tmpAction); + if (MODES_ERROR_NONE != ret) { + ERR("plugin(%s) set() Fail(%d)", plugin->getName().c_str(), ret); + return ret; + } + if (tmpAction) + tmpAction->setChangedCallback(valueChangedCallback, this); + + piAction = tmpAction; + return MODES_ERROR_NONE; } int applyOneShot() override { - return applyImpl(true, getType() == Action::ActionType::ASYNC); + RETVM_IF(NULL == plugin, MODES_ERROR_NO_DATA, "Action(%s) : No plugin", ruleName.c_str()); + + int pos = ruleName.find_first_of("."); + PluginAction *tmpAction = nullptr; + int ret = plugin->set(ruleName.substr(pos + 1), value, &tmpAction); + if (MODES_ERROR_NONE != ret) { + ERR("plugin(%s) set() Fail(%d)", plugin->getName().c_str(), ret); + return ret; + } + + piAction = tmpAction; + return MODES_ERROR_NONE; } void undo() override { RETM_IF(NULL == plugin, "Action(%s) : No plugin", ruleName.c_str()); + + //It control multithread condition + if (Action::ActionType::ASYNC == type) + undoing = true; + RETM_IF(NULL == piAction, "Plugin(%s) : No piAction(%s)", plugin->getName().c_str(), ruleName.c_str()); if (isChanged) { @@ -135,53 +173,12 @@ private: bool operator()(const std::string &s1, const std::string &s2) const { return std::lexicographical_compare(s1.begin(), s1.end(), - s2.begin(), s2.end(), Compare()); + s2.begin(), s2.end(), Compare()); } }; - int applyImpl(bool isOneShot, bool asyncFlag) - { - RETVM_IF(NULL == plugin, MODES_ERROR_NO_DATA, "Action(%s) : No plugin", ruleName.c_str()); - - int pos = ruleName.find_first_of("."); - if (asyncFlag) { - std::thread th1([](TAction * action, Plugin * plugin, std::string key, T value, bool isOneShot) { - PluginAction *tmpAction = nullptr; - int ret = plugin->set(key, value, &tmpAction); - if (MODES_ERROR_NONE != ret) { - ERR("plugin(%s) set() Fail(%d)", plugin->getName().c_str(), ret); - return; - } - - // mode class and action class are destroyed at applyOneShot. - // so need to skip register callback function. - if (!isOneShot) { - if (tmpAction != nullptr) - tmpAction->setChangedCallback(action->valueChangedCallback, action); - - action->piAction = tmpAction; - } - return; - }, this, plugin, ruleName.substr(pos + 1), value, isOneShot); - th1.detach(); - } else { - PluginAction *tmpAction = nullptr; - - int ret = plugin->set(ruleName.substr(pos + 1), value, &tmpAction); - if (MODES_ERROR_NONE != ret) { - ERR("plugin(%s) set() Fail(%d)", plugin->getName().c_str(), ret); - return ret; - } - - if (!isOneShot && tmpAction != nullptr) - tmpAction->setChangedCallback(valueChangedCallback, this); - - piAction = tmpAction; - } - return MODES_ERROR_NONE; - } - std::map valueAliases; + bool undoing; T value{}; }; MODES_NAMESPACE_END diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 994eb02..d68daeb 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -25,6 +25,12 @@ ADD_EXECUTABLE(${GTEST_CLIENT} ${SRC} ${GTEST_CLIENT_SRCS}) TARGET_LINK_LIBRARIES(${GTEST_CLIENT} ${CLIENT} ${gtest_pkgs_LIBRARIES}) INSTALL(TARGETS ${GTEST_CLIENT} DESTINATION ${TEST_INSTALL_DIR}) #=======================================================================================# +SET(GTEST_CLIENT "modes-gtest-async") +SET(GTEST_CLIENT_SRCS modes_test_async.cpp) +ADD_EXECUTABLE(${GTEST_CLIENT} ${SRC} ${GTEST_CLIENT_SRCS}) +TARGET_LINK_LIBRARIES(${GTEST_CLIENT} ${CLIENT} ${gtest_pkgs_LIBRARIES}) +INSTALL(TARGETS ${GTEST_CLIENT} DESTINATION ${TEST_INSTALL_DIR}) +#=======================================================================================# SET(TEST_APPLY_MODE "modes-mode-test") SET(TEST_APPLY_MODE_SRC modes_mode_test.c) ADD_EXECUTABLE(${TEST_APPLY_MODE} ${TEST_APPLY_MODE_SRC}) diff --git a/unittest/modes-gtest-run.sh b/unittest/modes-gtest-run.sh index daa57b5..941454f 100755 --- a/unittest/modes-gtest-run.sh +++ b/unittest/modes-gtest-run.sh @@ -28,6 +28,7 @@ else # on Target cp $DATA_DIR/schema/*.xsd ./ ./modes-gtest-client ./modes-gtest-noti + ./modes-gtest-async fi ./modes-gtest-rulemgr diff --git a/unittest/modes_test_async.cpp b/unittest/modes_test_async.cpp new file mode 100644 index 0000000..9752d7c --- /dev/null +++ b/unittest/modes_test_async.cpp @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2019 Samsung Electronics Co., Ltd All Rights Reserved + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include +#include +#include + +class AsyncTest : public ::testing::Test { +protected: + void SetUp() override + { + loop = g_main_loop_new(NULL, FALSE); + } + + void TearDown() override + { + g_main_loop_unref(loop); + loop = NULL; + } + + static gboolean ModeIdler(gpointer data) + { + int ret; + ret = modes_apply_mode((char*)data); + EXPECT_EQ(MODES_ERROR_NONE, ret); + sleep(1); + ret = modes_undo_mode((char*)data); + EXPECT_EQ(MODES_ERROR_NONE, ret); + + g_main_loop_quit(loop); + return G_SOURCE_REMOVE; + } + + static gboolean failIdler(gpointer data) + { + int ret; + ret = modes_apply_mode((char*)data); + EXPECT_EQ(MODES_ERROR_SYSTEM, ret); + sleep(1); + ret = modes_undo_mode((char*)data); + EXPECT_EQ(MODES_ERROR_NO_DATA, ret); + + g_main_loop_quit(loop); + return G_SOURCE_REMOVE; + } + + static GMainLoop *loop; +}; + +GMainLoop *AsyncTest::loop = NULL; + +TEST_F(AsyncTest, normalAsync) +{ + const char *modeName = "asyncEx1"; + modes_undo_mode(modeName); + g_idle_add(ModeIdler, (gpointer)modeName); + g_main_loop_run(loop); +} + +TEST_F(AsyncTest, oneshotAsync) +{ + const char *modeName = "asyncEx2"; + int ret = modes_apply_mode(modeName); + EXPECT_EQ(MODES_ERROR_NONE, ret); +} + +TEST_F(AsyncTest, normalAsyncFail) +{ + const char *modeName = "asyncFail1"; + modes_undo_mode(modeName); + g_idle_add(failIdler, (gpointer)modeName); + g_main_loop_run(loop); +} + +TEST_F(AsyncTest, oneshotAsyncFail) +{ + const char *modeName = "asyncFail2"; + g_idle_add(failIdler, (gpointer)modeName); + g_main_loop_run(loop); +} diff --git a/unittest/modes_test_noti.cpp b/unittest/modes_test_noti.cpp index bad0789..0acba25 100644 --- a/unittest/modes_test_noti.cpp +++ b/unittest/modes_test_noti.cpp @@ -36,7 +36,6 @@ protected: int ret = modes_undo_mode((const char*)data); EXPECT_EQ(MODES_ERROR_NONE, ret); - g_main_loop_quit(loop); return G_SOURCE_REMOVE; } @@ -49,7 +48,8 @@ protected: std::cout << "state : " << state << std::endl; EXPECT_EQ(expectedState, state); - g_main_loop_quit(loop); + if (0 == expectedState) + g_main_loop_quit(loop); return MODES_ERROR_NONE; }