Implement pthread_attr_getinheritsched/pthread_attr_setinheritsched.
Historically, Android defaulted to EXPLICIT but with a special case
because SCHED_NORMAL/priority 0 was awkward. Because the code couldn't
actually tell whether SCHED_NORMAL/priority 0 was a genuine attempt to
explicitly set those attributes (because the parent thread is SCHED_FIFO,
say) or just because the pthread_attr_t was left at its defaults.
Now we support INHERIT, we could call sched_getscheduler to see whether
we actually need to call sched_setscheduler, but since the major cost
is the fixed syscall overhead, we may as well just conservatively
call sched_setscheduler and let the kernel decide whether it's a
no-op. (Especially because we'd then have to add both sched_getscheduler
and sched_setscheduler to any seccomp filter.)
Platform code (or app code that only needs to support >= P) can actually
add a call to pthread_attr_setinheritsched to say that they just want
to inherit (if they know that none of their threads actually mess with
scheduler attributes at all), which will save them a sched_setscheduler
call except in the doubly-special case of SCHED_RESET_ON_FORK (which we
do handle).
An alternative would be "make pthread_attr_setschedparams and
pthread_attr_setschedprio set EXPLICIT and change the platform default
to INHERIT", but even though I can only think of weird pathological
examples where anyone would notice that change, that behavior -- of
pthread_attr_setschedparams/pthread_attr_setschedprio overriding an
earlier call to pthread_attr_setinheritsched -- isn't allowed by POSIX
(whereas defaulting to EXPLICIT is).
If we have a lot of trouble with this change in the app compatibility
testing phase, though, we'll want to reconsider this decision!
-*-
This change also removes a comment about setting the scheduler attributes
in main_thread because we'd have to actually keep them up to date,
and it's not clear that doing so would be worth the trouble.
Also make async_safe_format_log preserve errno so we don't have to be
so careful around it.
Bug: http://b/67471710
Test: ran tests
Change-Id: Idd026c4ce78a536656adcb57aa2e7b2c616eeddf
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index 85978bd..fb2a679 100755
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -1527,7 +1527,7 @@
ASSERT_EQ(0, pthread_create(&t, NULL,
reinterpret_cast<void* (*)(void*)>(pthread_attr_getstack_18908062_helper),
NULL));
- pthread_join(t, NULL);
+ ASSERT_EQ(0, pthread_join(t, NULL));
}
#if defined(__BIONIC__)
@@ -1559,7 +1559,7 @@
// Release the other thread and wait for it to exit.
pthread_mutex_unlock(&pthread_gettid_np_mutex);
- pthread_join(t, NULL);
+ ASSERT_EQ(0, pthread_join(t, NULL));
ASSERT_EQ(t_gettid_result, t_pthread_gettid_np_result);
#else
@@ -1600,7 +1600,7 @@
TEST(pthread, pthread_cleanup_push__pthread_cleanup_pop) {
pthread_t t;
ASSERT_EQ(0, pthread_create(&t, NULL, PthreadCleanupStartRoutine, NULL));
- pthread_join(t, NULL);
+ ASSERT_EQ(0, pthread_join(t, NULL));
ASSERT_EQ(2U, cleanup_counter);
}
@@ -2113,13 +2113,22 @@
ASSERT_EQ(0, pthread_spin_destroy(&lock));
}
-TEST(pthread, pthread_attr_setdetachstate) {
+TEST(pthread, pthread_attr_getdetachstate__pthread_attr_setdetachstate) {
pthread_attr_t attr;
ASSERT_EQ(0, pthread_attr_init(&attr));
+ int state;
ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED));
+ ASSERT_EQ(0, pthread_attr_getdetachstate(&attr, &state));
+ ASSERT_EQ(PTHREAD_CREATE_DETACHED, state);
+
ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE));
+ ASSERT_EQ(0, pthread_attr_getdetachstate(&attr, &state));
+ ASSERT_EQ(PTHREAD_CREATE_JOINABLE, state);
+
ASSERT_EQ(EINVAL, pthread_attr_setdetachstate(&attr, 123));
+ ASSERT_EQ(0, pthread_attr_getdetachstate(&attr, &state));
+ ASSERT_EQ(PTHREAD_CREATE_JOINABLE, state);
}
TEST(pthread, pthread_create__mmap_failures) {
@@ -2168,3 +2177,107 @@
TEST(pthread, pthread_setschedprio) {
ASSERT_EQ(EINVAL, pthread_setschedprio(pthread_self(), INT_MIN));
}
+
+TEST(pthread, pthread_attr_getinheritsched__pthread_attr_setinheritsched) {
+ pthread_attr_t attr;
+ ASSERT_EQ(0, pthread_attr_init(&attr));
+
+ int state;
+ ASSERT_EQ(0, pthread_attr_setinheritsched(&attr, PTHREAD_INHERIT_SCHED));
+ ASSERT_EQ(0, pthread_attr_getinheritsched(&attr, &state));
+ ASSERT_EQ(PTHREAD_INHERIT_SCHED, state);
+
+ ASSERT_EQ(0, pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED));
+ ASSERT_EQ(0, pthread_attr_getinheritsched(&attr, &state));
+ ASSERT_EQ(PTHREAD_EXPLICIT_SCHED, state);
+
+ ASSERT_EQ(EINVAL, pthread_attr_setinheritsched(&attr, 123));
+ ASSERT_EQ(0, pthread_attr_getinheritsched(&attr, &state));
+ ASSERT_EQ(PTHREAD_EXPLICIT_SCHED, state);
+}
+
+TEST(pthread, pthread_attr_setinheritsched__PTHREAD_INHERIT_SCHED__PTHREAD_EXPLICIT_SCHED) {
+ pthread_attr_t attr;
+ ASSERT_EQ(0, pthread_attr_init(&attr));
+
+ // If we set invalid scheduling attributes but choose to inherit, everything's fine...
+ sched_param param = { .sched_priority = sched_get_priority_max(SCHED_FIFO) + 1 };
+ ASSERT_EQ(0, pthread_attr_setschedparam(&attr, ¶m));
+ ASSERT_EQ(0, pthread_attr_setschedpolicy(&attr, SCHED_FIFO));
+ ASSERT_EQ(0, pthread_attr_setinheritsched(&attr, PTHREAD_INHERIT_SCHED));
+
+ pthread_t t;
+ ASSERT_EQ(0, pthread_create(&t, &attr, IdFn, nullptr));
+ ASSERT_EQ(0, pthread_join(t, nullptr));
+
+ // If we ask to use them, though...
+ ASSERT_EQ(0, pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED));
+ ASSERT_EQ(EINVAL, pthread_create(&t, &attr, IdFn, nullptr));
+}
+
+TEST(pthread, pthread_attr_setinheritsched_PTHREAD_INHERIT_SCHED_takes_effect) {
+ sched_param param = { .sched_priority = sched_get_priority_min(SCHED_FIFO) };
+ int rc = pthread_setschedparam(pthread_self(), SCHED_FIFO, ¶m);
+ if (rc == EPERM) {
+ GTEST_LOG_(INFO) << "pthread_setschedparam failed with EPERM, skipping test\n";
+ return;
+ }
+ ASSERT_EQ(0, rc);
+
+ pthread_attr_t attr;
+ ASSERT_EQ(0, pthread_attr_init(&attr));
+ ASSERT_EQ(0, pthread_attr_setinheritsched(&attr, PTHREAD_INHERIT_SCHED));
+
+ pthread_t t;
+ ASSERT_EQ(0, pthread_create(&t, &attr, IdFn, nullptr));
+ int actual_policy;
+ sched_param actual_param;
+ ASSERT_EQ(0, pthread_getschedparam(t, &actual_policy, &actual_param));
+ ASSERT_EQ(SCHED_FIFO, actual_policy);
+ ASSERT_EQ(0, pthread_join(t, nullptr));
+}
+
+TEST(pthread, pthread_attr_setinheritsched_PTHREAD_EXPLICIT_SCHED_takes_effect) {
+ sched_param param = { .sched_priority = sched_get_priority_min(SCHED_FIFO) };
+ int rc = pthread_setschedparam(pthread_self(), SCHED_FIFO, ¶m);
+ if (rc == EPERM) {
+ GTEST_LOG_(INFO) << "pthread_setschedparam failed with EPERM, skipping test\n";
+ return;
+ }
+ ASSERT_EQ(0, rc);
+
+ pthread_attr_t attr;
+ ASSERT_EQ(0, pthread_attr_init(&attr));
+ ASSERT_EQ(0, pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED));
+ ASSERT_EQ(0, pthread_attr_setschedpolicy(&attr, SCHED_OTHER));
+
+ pthread_t t;
+ ASSERT_EQ(0, pthread_create(&t, &attr, IdFn, nullptr));
+ int actual_policy;
+ sched_param actual_param;
+ ASSERT_EQ(0, pthread_getschedparam(t, &actual_policy, &actual_param));
+ ASSERT_EQ(SCHED_OTHER, actual_policy);
+ ASSERT_EQ(0, pthread_join(t, nullptr));
+}
+
+TEST(pthread, pthread_attr_setinheritsched__takes_effect_despite_SCHED_RESET_ON_FORK) {
+ sched_param param = { .sched_priority = sched_get_priority_min(SCHED_FIFO) };
+ int rc = pthread_setschedparam(pthread_self(), SCHED_FIFO | SCHED_RESET_ON_FORK, ¶m);
+ if (rc == EPERM) {
+ GTEST_LOG_(INFO) << "pthread_setschedparam failed with EPERM, skipping test\n";
+ return;
+ }
+ ASSERT_EQ(0, rc);
+
+ pthread_attr_t attr;
+ ASSERT_EQ(0, pthread_attr_init(&attr));
+ ASSERT_EQ(0, pthread_attr_setinheritsched(&attr, PTHREAD_INHERIT_SCHED));
+
+ pthread_t t;
+ ASSERT_EQ(0, pthread_create(&t, &attr, IdFn, nullptr));
+ int actual_policy;
+ sched_param actual_param;
+ ASSERT_EQ(0, pthread_getschedparam(t, &actual_policy, &actual_param));
+ ASSERT_EQ(SCHED_FIFO | SCHED_RESET_ON_FORK, actual_policy);
+ ASSERT_EQ(0, pthread_join(t, nullptr));
+}