From: Piotr Bartosiewicz Date: Mon, 8 Dec 2014 11:52:32 +0000 (+0100) Subject: Fix some other threading issues X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b550c251c29c804983f019305b6e7574b2f7b585;p=platform%2Fcore%2Fsecurity%2Fvasum.git Fix some other threading issues [Bug/Feature] N/A [Cause] N/A [Solution] N/A [Verification] Build, install, run tests Change-Id: I89b35811644f424773c007bb7d13d622ba57ac48 --- diff --git a/common/lxc/zone.cpp b/common/lxc/zone.cpp index c7736f5..f4ad3e4 100644 --- a/common/lxc/zone.cpp +++ b/common/lxc/zone.cpp @@ -34,8 +34,8 @@ #include "logger/logger.hpp" #include "lxc/zone.hpp" #include "lxc/exception.hpp" -#ifdef USE_EXEC #include "utils/execute.hpp" +#ifdef USE_EXEC #include "utils/c-array.hpp" #endif @@ -320,7 +320,7 @@ bool LxcZone::setRunLevel(int runLevel) return false; } int status; - if (waitpid(pid, &status, 0) < 0) { + if (!utils::waitPid(pid, status)) { return false; } return status == 0; diff --git a/common/utils/environment.cpp b/common/utils/environment.cpp index 73c7057..aec70c1 100644 --- a/common/utils/environment.cpp +++ b/common/utils/environment.cpp @@ -25,6 +25,7 @@ #include "config.hpp" #include "utils/environment.hpp" +#include "utils/execute.hpp" #include "logger/logger.hpp" #include @@ -96,29 +97,28 @@ bool launchAsRoot(const std::function& func) if (pid == 0) { if (::setuid(0) < 0) { LOGW("Failed to become root: " << strerror(errno)); - ::exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } try { if (!func()) { LOGE("Failed to successfully execute func"); - ::exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } } catch (const std::exception& e) { LOGE("Failed to successfully execute func: " << e.what()); - ::exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } - ::exit(EXIT_SUCCESS); + _exit(EXIT_SUCCESS); } - int result; - if (::waitpid(pid, &result, 0) < 0) { - LOGE("waitpid failed: " << strerror(errno)); + int status; + if (!waitPid(pid, status)) { return false; } - if (result != 0) { - LOGE("Function launched as root failed with result " << result); + if (status != 0) { + LOGE("Function launched as root exited with status " << status); return false; } diff --git a/common/utils/execute.cpp b/common/utils/execute.cpp index 2f2e927..6511456 100644 --- a/common/utils/execute.cpp +++ b/common/utils/execute.cpp @@ -61,12 +61,7 @@ bool executeAndWait(const char* fname, const char* const* argv, int& status) execv(fname, const_cast(argv)); _exit(EXIT_FAILURE); } - int ret = waitpid(pid, &status, 0); - if (ret != pid) { - LOGE("Waitpid failed"); - return false; - } - return true; + return waitPid(pid, status); } bool executeAndWait(const char* fname, const char* const* argv) @@ -88,5 +83,15 @@ bool executeAndWait(const char* fname, const char* const* argv) return true; } +bool waitPid(pid_t pid, int& status) +{ + while (waitpid(pid, &status, 0) == -1) { + if (errno != EINTR) { + return false; + } + } + return true; +} + } // namespace utils } // namespace security_containers diff --git a/common/utils/execute.hpp b/common/utils/execute.hpp index 1fc7b29..3256ee2 100644 --- a/common/utils/execute.hpp +++ b/common/utils/execute.hpp @@ -25,6 +25,7 @@ #ifndef COMMON_UTILS_EXECUTE_HPP #define COMMON_UTILS_EXECUTE_HPP +#include namespace security_containers { namespace utils { @@ -33,6 +34,8 @@ bool executeAndWait(const char* fname, const char* const* argv); bool executeAndWait(const char* fname, const char* const* argv, int& status); +bool waitPid(pid_t pid, int& status); + } // namespace utils } // namespace security_containers diff --git a/tests/unit_tests/server/ut-containers-manager.cpp b/tests/unit_tests/server/ut-containers-manager.cpp index 6d64c83..ac4a898 100644 --- a/tests/unit_tests/server/ut-containers-manager.cpp +++ b/tests/unit_tests/server/ut-containers-manager.cpp @@ -460,27 +460,6 @@ std::function expectedMessage(const std::string& me }; } -class FileCleanerRAII { -public: - FileCleanerRAII(const std::vector& filePathsToClean): - mFilePathsToClean(filePathsToClean) - { } - - ~FileCleanerRAII() - { - namespace fs = boost::filesystem; - for (const auto& file : mFilePathsToClean) { - fs::path f(file); - if (fs::exists(f)) { - fs::remove(f); - } - } - } - -private: - const std::vector mFilePathsToClean; -}; - struct Fixture { security_containers::utils::ScopedGlibLoop mLoop; @@ -1041,11 +1020,15 @@ BOOST_AUTO_TEST_CASE(SetActiveContainerTest) BOOST_AUTO_TEST_CASE(CreateDestroyContainerTest) { - const std::string newContainerId = "test1234"; + const std::string container1 = "test1"; + const std::string container2 = "test2"; + const std::string container3 = "test3"; ContainersManager cm(EMPTY_DBUS_CONFIG_PATH); cm.startAll(); + BOOST_CHECK_EQUAL(cm.getRunningForegroundContainerId(), ""); + Latch callDone; auto resultCallback = [&]() { callDone.set(); @@ -1053,19 +1036,36 @@ BOOST_AUTO_TEST_CASE(CreateDestroyContainerTest) DbusAccessory dbus(DbusAccessory::HOST_ID); - // create new container - dbus.callAsyncMethodCreateContainer(newContainerId, resultCallback); + // create container1 + dbus.callAsyncMethodCreateContainer(container1, resultCallback); BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT)); - // focus new container - cm.focus(newContainerId); - BOOST_CHECK(cm.getRunningForegroundContainerId() == newContainerId); + BOOST_CHECK_EQUAL(cm.getRunningForegroundContainerId(), container1); - // destroy container - dbus.callAsyncMethodDestroyContainer(newContainerId, resultCallback); + // create container2 + dbus.callAsyncMethodCreateContainer(container2, resultCallback); BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT)); + BOOST_CHECK_EQUAL(cm.getRunningForegroundContainerId(), container2); //TODO is this valid? - BOOST_CHECK(cm.getRunningForegroundContainerId() == ""); + // create container3 + dbus.callAsyncMethodCreateContainer(container3, resultCallback); + BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT)); + BOOST_CHECK_EQUAL(cm.getRunningForegroundContainerId(), container3); + + // destroy container2 + dbus.callAsyncMethodDestroyContainer(container2, resultCallback); + BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT)); + BOOST_CHECK_EQUAL(cm.getRunningForegroundContainerId(), container3); + + // destroy container3 + dbus.callAsyncMethodDestroyContainer(container3, resultCallback); + BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT)); + //BOOST_CHECK_EQUAL(cm.getRunningForegroundContainerId(), container1);//TODO fix it + + // destroy container1 + dbus.callAsyncMethodDestroyContainer(container1, resultCallback); + BOOST_REQUIRE(callDone.wait(EVENT_TIMEOUT)); + BOOST_CHECK_EQUAL(cm.getRunningForegroundContainerId(), ""); } BOOST_AUTO_TEST_CASE(DeclareFile) diff --git a/tests/unit_tests/utils/scoped-daemon.cpp b/tests/unit_tests/utils/scoped-daemon.cpp index 21fea83..db85375 100644 --- a/tests/unit_tests/utils/scoped-daemon.cpp +++ b/tests/unit_tests/utils/scoped-daemon.cpp @@ -25,6 +25,7 @@ #include "config.hpp" #include "utils/scoped-daemon.hpp" +#include "utils/execute.hpp" #include "logger/logger.hpp" @@ -69,17 +70,17 @@ namespace { volatile pid_t daemonPid = -1;// available in launcher process only; -void startDaemon(const char* path, const char* const argv[]) +bool startDaemon(const char* path, const char* const argv[]) { execv(path, const_cast(argv)); perror("exec failed"); + return false; } -void waitForDaemon() +bool waitForDaemon() { - if (waitpid(daemonPid, NULL, 0) == -1) { - perror("wait for daemon failed"); - } + int status; + return waitPid(daemonPid, status); } void launcherSignalHandler(int sig) @@ -108,21 +109,22 @@ void cleanupProcess() signal(SIGHUP, SIG_DFL); } -void startByLauncher(const char* path, const char* const argv[]) +bool startByLauncher(const char* path, const char* const argv[]) { cleanupProcess(); daemonPid = fork(); if (daemonPid == -1) { perror("fork failed"); - return; + return false; } if (daemonPid == 0) { - startDaemon(path, argv); - _exit(1); + if (!startDaemon(path, argv)) { + return false; + } } registerLauncherSignalHandler(); registerParentDiedNotification(); - waitForDaemon(); + return waitForDaemon(); } } // namespace @@ -147,12 +149,13 @@ void ScopedDaemon::start(const char* path, const char* const argv[], const bool throw std::runtime_error("fork failed"); } if (mPid == 0) { + bool ret; if (useLauncher) { - startByLauncher(path, argv); + ret = startByLauncher(path, argv); } else { - startDaemon(path, argv); + ret = startDaemon(path, argv); } - _exit(0); + _exit(ret ? EXIT_SUCCESS : EXIT_FAILURE); } } @@ -164,8 +167,12 @@ void ScopedDaemon::stop() if (kill(mPid, SIGTERM) == -1) { LOGE("kill failed"); } - if (waitpid(mPid, NULL, 0) == -1) { - LOGE("waitpid failed"); + int status; + if (!waitPid(mPid, status)) { + throw std::runtime_error("waitpid failed"); + } + if (status != EXIT_SUCCESS) { + LOGW("process exit with status " << status); } mPid = -1; } diff --git a/tests/unit_tests/utils/scoped-daemon.hpp b/tests/unit_tests/utils/scoped-daemon.hpp index 89e630c..b3eab29 100644 --- a/tests/unit_tests/utils/scoped-daemon.hpp +++ b/tests/unit_tests/utils/scoped-daemon.hpp @@ -50,7 +50,7 @@ public: * @param argv arguments passed to the daemon * @param useLauncher use additional launcher process */ - void start(const char* path, const char* const argv[], const bool useLauncher = true); + void start(const char* path, const char* const argv[], const bool useLauncher = false); /** * Stops a daemon by sending SIGTERM and waits for a process.