Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(315)

Unified Diff: base/threading/platform_thread_unittest.cc

Issue 1193303002: base/threading: restrict to set only current thread priority (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: review #48 Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/threading/platform_thread_posix.cc ('k') | base/threading/platform_thread_win.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/threading/platform_thread_unittest.cc
diff --git a/base/threading/platform_thread_unittest.cc b/base/threading/platform_thread_unittest.cc
index c4b3d5d7ecc6092a89ca519800b55f92d3fd77da..1ac08a77ae2dea6836d60f0b8ad69104419ddd9e 100644
--- a/base/threading/platform_thread_unittest.cc
+++ b/base/threading/platform_thread_unittest.cc
@@ -8,7 +8,10 @@
#include "base/threading/platform_thread.h"
#include "testing/gtest/include/gtest/gtest.h"
-#if defined(OS_WIN)
+#if defined(OS_POSIX)
+#include <sys/types.h>
+#include <unistd.h>
+#elif defined(OS_WIN)
#include <windows.h>
#endif
@@ -16,6 +19,8 @@ namespace base {
// Trivial tests that thread runs and doesn't crash on create and join ---------
+namespace {
+
class TrivialThread : public PlatformThread::Delegate {
public:
TrivialThread() : did_run_(false) {}
@@ -30,6 +35,8 @@ class TrivialThread : public PlatformThread::Delegate {
DISALLOW_COPY_AND_ASSIGN(TrivialThread);
};
+} // namespace
+
TEST(PlatformThreadTest, Trivial) {
TrivialThread thread;
PlatformThreadHandle handle;
@@ -56,11 +63,13 @@ TEST(PlatformThreadTest, TrivialTimesTen) {
// Tests of basic thread functions ---------------------------------------------
+namespace {
+
class FunctionTestThread : public PlatformThread::Delegate {
public:
FunctionTestThread()
: thread_id_(kInvalidThreadId),
- thread_started_(true, false),
+ termination_ready_(true, false),
terminate_thread_(true, false),
done_(false) {}
~FunctionTestThread() override {
@@ -70,8 +79,9 @@ class FunctionTestThread : public PlatformThread::Delegate {
<< "WaitableEvent blocking the underlying thread's main.";
}
- // Grabs |thread_id_|, signals |thread_started_|, and then waits for
- // |terminate_thread_| to be signaled before exiting.
+ // Grabs |thread_id_|, runs an optional test on that thread, signals
+ // |termination_ready_|, and then waits for |terminate_thread_| to be
+ // signaled before exiting.
void ThreadMain() override {
thread_id_ = PlatformThread::CurrentId();
EXPECT_NE(thread_id_, kInvalidThreadId);
@@ -79,39 +89,46 @@ class FunctionTestThread : public PlatformThread::Delegate {
// Make sure that the thread ID is the same across calls.
EXPECT_EQ(thread_id_, PlatformThread::CurrentId());
- thread_started_.Signal();
+ // Run extra tests.
+ RunTest();
+ termination_ready_.Signal();
terminate_thread_.Wait();
done_ = true;
}
PlatformThreadId thread_id() const {
- EXPECT_TRUE(thread_started_.IsSignaled()) << "Thread ID still unknown";
+ EXPECT_TRUE(termination_ready_.IsSignaled()) << "Thread ID still unknown";
return thread_id_;
}
bool IsRunning() const {
- return thread_started_.IsSignaled() && !done_;
+ return termination_ready_.IsSignaled() && !done_;
}
- // Blocks until this thread is started.
- void WaitForThreadStart() { thread_started_.Wait(); }
+ // Blocks until this thread is started and ready to be terminated.
+ void WaitForTerminationReady() { termination_ready_.Wait(); }
- // Mark this thread for termination (callers must then join this thread to be
+ // Marks this thread for termination (callers must then join this thread to be
// guaranteed of termination).
void MarkForTermination() { terminate_thread_.Signal(); }
private:
+ // Runs an optional test on the newly created thread.
+ virtual void RunTest() {}
+
PlatformThreadId thread_id_;
- mutable WaitableEvent thread_started_;
+ mutable WaitableEvent termination_ready_;
WaitableEvent terminate_thread_;
bool done_;
DISALLOW_COPY_AND_ASSIGN(FunctionTestThread);
};
+} // namespace
+
TEST(PlatformThreadTest, Function) {
PlatformThreadId main_thread_id = PlatformThread::CurrentId();
@@ -120,7 +137,7 @@ TEST(PlatformThreadTest, Function) {
ASSERT_FALSE(thread.IsRunning());
ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
- thread.WaitForThreadStart();
+ thread.WaitForTerminationReady();
ASSERT_TRUE(thread.IsRunning());
EXPECT_NE(thread.thread_id(), main_thread_id);
@@ -144,7 +161,7 @@ TEST(PlatformThreadTest, FunctionTimesTen) {
for (size_t n = 0; n < arraysize(thread); n++)
ASSERT_TRUE(PlatformThread::Create(0, &thread[n], &handle[n]));
for (size_t n = 0; n < arraysize(thread); n++)
- thread[n].WaitForThreadStart();
+ thread[n].WaitForTerminationReady();
for (size_t n = 0; n < arraysize(thread); n++) {
ASSERT_TRUE(thread[n].IsRunning());
@@ -170,106 +187,86 @@ TEST(PlatformThreadTest, FunctionTimesTen) {
namespace {
const ThreadPriority kThreadPriorityTestValues[] = {
-// Disable non-normal priority toggling on POSIX as it appears to be broken
-// (http://crbug.com/468793). This is prefered to disabling the tests altogether
-// on POSIX as it at least provides coverage for running this code under
-// "normal" priority.
-#if !defined(OS_POSIX)
- ThreadPriority::DISPLAY,
+// The order should be higher to lower to cover as much cases as possible on
+// Linux trybots running without CAP_SYS_NICE permission.
+#if !defined(OS_ANDROID)
+ // PlatformThread::GetCurrentThreadPriority() on Android does not support
+ // REALTIME_AUDIO case. See http://crbug.com/505474.
ThreadPriority::REALTIME_AUDIO,
- // Keep BACKGROUND second to last to test backgrounding from other
- // priorities.
+#endif
+ ThreadPriority::DISPLAY,
+ // This redundant BACKGROUND priority is to test backgrounding from other
+ // priorities, and unbackgrounding.
ThreadPriority::BACKGROUND,
-#endif // !defined(OS_POSIX)
- // Keep NORMAL last to test unbackgrounding.
- ThreadPriority::NORMAL
-};
-
-} // namespace
-
-// Test changing another thread's priority.
-// NOTE: This test is partially disabled on POSIX, see note above and
-// http://crbug.com/468793.
-TEST(PlatformThreadTest, ThreadPriorityOtherThread) {
- PlatformThreadHandle current_handle(PlatformThread::CurrentHandle());
-
- // Confirm that the current thread's priority is as expected.
- EXPECT_EQ(ThreadPriority::NORMAL,
- PlatformThread::GetThreadPriority(current_handle));
-
- // Create a test thread.
- FunctionTestThread thread;
- PlatformThreadHandle handle;
- ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
- thread.WaitForThreadStart();
- EXPECT_NE(thread.thread_id(), kInvalidThreadId);
- EXPECT_NE(thread.thread_id(), PlatformThread::CurrentId());
-
- // New threads should get normal priority by default.
- EXPECT_EQ(ThreadPriority::NORMAL, PlatformThread::GetThreadPriority(handle));
-
- // Toggle each supported priority on the test thread and confirm it only
- // affects it (and not the current thread).
- for (size_t i = 0; i < arraysize(kThreadPriorityTestValues); ++i) {
- SCOPED_TRACE(i);
+ ThreadPriority::NORMAL,
+ ThreadPriority::BACKGROUND};
+
+bool IsBumpingPriorityAllowed() {
+#if defined(OS_POSIX)
+ // Only root can raise thread priority on POSIX environment. On Linux, users
+ // who have CAP_SYS_NICE permission also can raise the thread priority, but
+ // libcap.so would be needed to check the capability.
+ return geteuid() == 0;
+#else
+ return true;
+#endif
+}
- // Alter and verify the test thread's priority.
- PlatformThread::SetThreadPriority(handle, kThreadPriorityTestValues[i]);
- EXPECT_EQ(kThreadPriorityTestValues[i],
- PlatformThread::GetThreadPriority(handle));
+class ThreadPriorityTestThread : public FunctionTestThread {
+ public:
+ ThreadPriorityTestThread() = default;
+ ~ThreadPriorityTestThread() override = default;
- // Make sure the current thread was otherwise unaffected.
+ private:
+ void RunTest() override {
+ // Confirm that the current thread's priority is as expected.
EXPECT_EQ(ThreadPriority::NORMAL,
- PlatformThread::GetThreadPriority(current_handle));
+ PlatformThread::GetCurrentThreadPriority());
+
+ // Toggle each supported priority on the current thread and confirm it
+ // affects it.
+ const bool bumping_priority_allowed = IsBumpingPriorityAllowed();
+ for (size_t i = 0; i < arraysize(kThreadPriorityTestValues); ++i) {
+ SCOPED_TRACE(i);
+ if (!bumping_priority_allowed &&
+ kThreadPriorityTestValues[i] >
+ PlatformThread::GetCurrentThreadPriority()) {
+ continue;
+ }
+
+ // Alter and verify the current thread's priority.
+ PlatformThread::SetCurrentThreadPriority(kThreadPriorityTestValues[i]);
+ EXPECT_EQ(kThreadPriorityTestValues[i],
+ PlatformThread::GetCurrentThreadPriority());
+ }
}
- thread.MarkForTermination();
- PlatformThread::Join(handle);
-}
+ DISALLOW_COPY_AND_ASSIGN(ThreadPriorityTestThread);
+};
-// Test changing the current thread's priority (which has different semantics on
-// some platforms).
-// NOTE: This test is partially disabled on POSIX, see note above and
-// http://crbug.com/468793.
-TEST(PlatformThreadTest, ThreadPriorityCurrentThread) {
- PlatformThreadHandle current_handle(PlatformThread::CurrentHandle());
+} // namespace
- // Confirm that the current thread's priority is as expected.
- EXPECT_EQ(ThreadPriority::NORMAL,
- PlatformThread::GetThreadPriority(current_handle));
+#if defined(OS_MACOSX)
+// PlatformThread::GetCurrentThreadPriority() is not implemented on OS X.
+#define MAYBE_ThreadPriorityCurrentThread DISABLED_ThreadPriorityCurrentThread
+#else
+#define MAYBE_ThreadPriorityCurrentThread ThreadPriorityCurrentThread
+#endif
- // Create a test thread for verification purposes only.
- FunctionTestThread thread;
+// Test changing a created thread's priority (which has different semantics on
+// some platforms).
+TEST(PlatformThreadTest, MAYBE_ThreadPriorityCurrentThread) {
+ ThreadPriorityTestThread thread;
PlatformThreadHandle handle;
- ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
- thread.WaitForThreadStart();
- EXPECT_NE(thread.thread_id(), kInvalidThreadId);
- EXPECT_NE(thread.thread_id(), PlatformThread::CurrentId());
-
- // Confirm that the new thread's priority is as expected.
- EXPECT_EQ(ThreadPriority::NORMAL, PlatformThread::GetThreadPriority(handle));
-
- // Toggle each supported priority on the current thread and confirm it only
- // affects it (and not the test thread).
- for (size_t i = 0; i < arraysize(kThreadPriorityTestValues); ++i) {
- SCOPED_TRACE(i);
-
- // Alter and verify the current thread's priority.
- PlatformThread::SetThreadPriority(current_handle,
- kThreadPriorityTestValues[i]);
- EXPECT_EQ(kThreadPriorityTestValues[i],
- PlatformThread::GetThreadPriority(current_handle));
-
- // Make sure the test thread was otherwise unaffected.
- EXPECT_EQ(ThreadPriority::NORMAL,
- PlatformThread::GetThreadPriority(handle));
- }
- // Restore current thread priority for follow-up tests.
- PlatformThread::SetThreadPriority(current_handle, ThreadPriority::NORMAL);
+ ASSERT_FALSE(thread.IsRunning());
+ ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
+ thread.WaitForTerminationReady();
+ ASSERT_TRUE(thread.IsRunning());
thread.MarkForTermination();
PlatformThread::Join(handle);
+ ASSERT_FALSE(thread.IsRunning());
}
} // namespace base
« no previous file with comments | « base/threading/platform_thread_posix.cc ('k') | base/threading/platform_thread_win.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698