fixed lots of not-thread-safe things
Most of the things I fixed here were using libc functions that are
fundamentally not thread-safe.
diff --git a/aos/common/util/aos_strerror.cc b/aos/common/util/aos_strerror.cc
deleted file mode 100644
index d2b2ca6..0000000
--- a/aos/common/util/aos_strerror.cc
+++ /dev/null
@@ -1,42 +0,0 @@
-#include "aos/common/util/aos_strerror.h"
-
-#include <assert.h>
-#include <sys/types.h>
-#include <string.h>
-#include <stdio.h>
-
-#include "aos/linux_code/thread_local.h"
-
-// This code uses an overloaded function to handle the result from either
-// version of strerror_r correctly without needing a way to get the choice out
-// of the compiler/glibc/whatever explicitly.
-
-namespace {
-
-const size_t kBufferSize = 128;
-
-// Handle the result from the GNU version of strerror_r. It never fails, so
-// that's pretty easy...
-__attribute__((unused))
-char *aos_strerror_handle_result(int /*error*/, char *ret, char * /*buffer*/) {
- return ret;
-}
-
-// Handle the result from the POSIX version of strerror_r.
-__attribute__((unused))
-char *aos_strerror_handle_result(int error, int ret, char *buffer) {
- if (ret != 0) {
- assert(snprintf(buffer, kBufferSize, "Unknown error %d", error) > 0);
- }
- return buffer;
-}
-
-} // namespace
-
-char *aos_strerror(int error) {
- static AOS_THREAD_LOCAL char buffer[kBufferSize];
-
- // Call the overload for whichever version we're using.
- return aos_strerror_handle_result(
- error, strerror_r(error, buffer, sizeof(buffer)), buffer);
-}
diff --git a/aos/common/util/aos_strerror.h b/aos/common/util/aos_strerror.h
deleted file mode 100644
index 2fd6818..0000000
--- a/aos/common/util/aos_strerror.h
+++ /dev/null
@@ -1,21 +0,0 @@
-#ifndef AOS_COMMON_UTIL_AOS_STRERROR_H_
-#define AOS_COMMON_UTIL_AOS_STRERROR_H_
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-// Thread-safe version of strerror(3) (except it may change errno).
-//
-// Necessary because strerror_r(3) is such a mess (which version you get is
-// determined at compile time by black magic related to feature macro
-// definitions, compiler flags, glibc version, and even whether you're using g++
-// or clang++) and strerror_l(3) might not work if you end up with the magic
-// LC_GLOBAL_LOCALE locale.
-char *aos_strerror(int error);
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif // AOS_COMMON_UTIL_AOS_STRERROR_H_
diff --git a/aos/common/util/run_command.cc b/aos/common/util/run_command.cc
new file mode 100644
index 0000000..a4a19d8
--- /dev/null
+++ b/aos/common/util/run_command.cc
@@ -0,0 +1,73 @@
+#include "aos/common/util/run_command.h"
+
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#include "aos/common/logging/logging.h"
+
+namespace aos {
+namespace util {
+namespace {
+
+// RAII class to block SIGCHLD and then restore it on destruction.
+class BlockSIGCHLD {
+ public:
+ BlockSIGCHLD() {
+ sigset_t to_block;
+ sigemptyset(&to_block);
+ sigaddset(&to_block, SIGCHLD);
+ if (sigprocmask(SIG_BLOCK, &to_block, &original_blocked_) == -1) {
+ PLOG(FATAL, "sigprocmask(SIG_BLOCK, %p, %p) failed",
+ &to_block, &original_blocked_);
+ }
+ }
+ ~BlockSIGCHLD() {
+ if (sigprocmask(SIG_SETMASK, &original_blocked_, nullptr) == -1) {
+ PLOG(FATAL, "sigprocmask(SIG_SETMASK, %p, nullptr) failed",
+ &original_blocked_);
+ }
+ }
+
+ private:
+ sigset_t original_blocked_;
+};
+
+} // namespace
+
+int RunCommand(const char *command) {
+ BlockSIGCHLD blocker;
+ const pid_t pid = fork();
+ switch (pid) {
+ case 0: // in child
+ {
+ int new_stdin = open("/dev/null", O_RDONLY);
+ if (new_stdin == -1) _exit(127);
+ int new_stdout = open("/dev/null", O_WRONLY);
+ if (new_stdout == -1) _exit(127);
+ int new_stderr = open("/dev/null", O_WRONLY);
+ if (new_stderr == -1) _exit(127);
+ if (dup2(new_stdin, 0) != 0) _exit(127);
+ if (dup2(new_stdout, 1) != 1) _exit(127);
+ if (dup2(new_stderr, 2) != 2) _exit(127);
+ execl("/bin/sh", "sh", "-c", command, nullptr);
+ _exit(127);
+ }
+ case -1:
+ return -1;
+ default:
+ int stat;
+ while (waitpid(pid, &stat, 0) == -1) {
+ if (errno != EINTR) {
+ return -1;
+ }
+ }
+ return stat;
+ }
+}
+
+} // namespace util
+} // namespace aos
diff --git a/aos/common/util/run_command.h b/aos/common/util/run_command.h
new file mode 100644
index 0000000..d116481
--- /dev/null
+++ b/aos/common/util/run_command.h
@@ -0,0 +1,17 @@
+#ifndef AOS_COMMON_UTIL_RUN_COMMAND_H_
+#define AOS_COMMON_UTIL_RUN_COMMAND_H_
+
+namespace aos {
+namespace util {
+
+// Improved replacement for system(3). Doesn't block signals like system(3) and
+// is thread-safe. Also makes sure all 3 standard streams are /dev/null.
+//
+// This means that it passes command to `/bin/sh -c` and returns -1 or a status
+// like from wait(2).
+int RunCommand(const char *command);
+
+} // namespace util
+} // namespace aos
+
+#endif // AOS_COMMON_UTIL_RUN_COMMAND_H_
diff --git a/aos/common/util/run_command_test.cc b/aos/common/util/run_command_test.cc
new file mode 100644
index 0000000..b440e47
--- /dev/null
+++ b/aos/common/util/run_command_test.cc
@@ -0,0 +1,39 @@
+#include "aos/common/util/run_command.h"
+
+#include "gtest/gtest.h"
+
+namespace aos {
+namespace util {
+namespace testing {
+
+TEST(RunCommandTest, True) {
+ int result = RunCommand("true");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFEXITED(result));
+ EXPECT_EQ(0, WEXITSTATUS(result));
+}
+
+TEST(RunCommandTest, False) {
+ int result = RunCommand("false");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFEXITED(result));
+ EXPECT_EQ(1, WEXITSTATUS(result));
+}
+
+TEST(RunCommandTest, CommandNotFound) {
+ int result = RunCommand("ajflkjasdlfa");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFEXITED(result));
+ EXPECT_EQ(127, WEXITSTATUS(result));
+}
+
+TEST(RunCommandTest, KilledBySignal) {
+ int result = RunCommand("kill -QUIT $$");
+ ASSERT_NE(-1, result);
+ ASSERT_TRUE(WIFSIGNALED(result));
+ EXPECT_EQ(SIGQUIT, WTERMSIG(result));
+}
+
+} // namespace testing
+} // namespace util
+} // namespace aos
diff --git a/aos/common/util/util.gyp b/aos/common/util/util.gyp
index 9f3e1c6..7c0adf3 100644
--- a/aos/common/util/util.gyp
+++ b/aos/common/util/util.gyp
@@ -1,6 +1,28 @@
{
'targets': [
{
+ 'target_name': 'run_command',
+ 'type': 'static_library',
+ 'sources': [
+ 'run_command.cc',
+ ],
+ 'dependencies': [
+ '<(AOS)/build/aos.gyp:logging_interface',
+ ],
+ },
+ {
+ 'target_name': 'run_command_test',
+ 'type': 'executable',
+ 'sources': [
+ 'run_command_test.cc',
+ ],
+ 'dependencies': [
+ 'run_command',
+ '<(EXTERNALS):gtest',
+ '<(AOS)/build/aos.gyp:logging',
+ ],
+ },
+ {
'target_name': 'death_test_log_implementation',
'type': 'static_library',
'sources': [
@@ -14,13 +36,6 @@
],
},
{
- 'target_name': 'aos_strerror',
- 'type': 'static_library',
- 'sources': [
- 'aos_strerror.cc',
- ],
- },
- {
'target_name': 'inet_addr',
'type': 'static_library',
'sources': [