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.