diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c index 204b642fd..efd5429bc 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c @@ -108,7 +108,7 @@ void memcpy(uint8_t *dest, uint8_t *src, size_t n) { // // This queue is lock-less to be sure that any thread scheduled out // from CPU doesn't block others. -#define SPINNING_QUEUE_SIZE 128 +#define SPINNING_QUEUE_SIZE 384 // MAX_SPINNING_THREADS is half of SPINNING_QUEUE_SIZE to be sure that the tail // doesn't catch the head. More details are in spinning_queue_remove_first. @@ -120,6 +120,7 @@ void memcpy(uint8_t *dest, uint8_t *src, size_t n) { #define MAX_RE_ENQUEUE 2 struct spinning_queue { + uint32_t len; uint32_t start; uint32_t end; uint64_t start_times[SPINNING_QUEUE_SIZE]; @@ -134,19 +135,19 @@ static bool spinning_queue_push(uint8_t re_enqueue_times) __attribute__((warn_unused_result)); static bool spinning_queue_push(uint8_t re_enqueue_times) { struct spinning_queue *queue = __export_spinning_queue_addr; - uint32_t idx, start, end; + uint32_t idx, end, len; BUILD_BUG_ON(sizeof(struct spinning_queue) > SPINNING_QUEUE_MEM_SIZE); if (re_enqueue_times >= MAX_RE_ENQUEUE) { return false; } - end = atomic_add(&queue->end, 1); - start = atomic_load(&queue->start); - if (end - start > MAX_SPINNING_THREADS) { - atomic_sub(&queue->end, 1); + len = atomic_add(&queue->len, 1); + if (len > MAX_SPINNING_THREADS) { + atomic_sub(&queue->len, 1); return false; } + end = atomic_add(&queue->end, 1); idx = end - 1; atomic_store(&queue->num_times_re_enqueued[idx % SPINNING_QUEUE_SIZE], @@ -162,6 +163,7 @@ static void spinning_queue_pop() { struct spinning_queue *queue = __export_spinning_queue_addr; atomic_sub(&queue->end, 1); + atomic_sub(&queue->len, 1); } // spinning_queue_remove_first removes one thread from a queue that has been @@ -195,6 +197,7 @@ static bool spinning_queue_remove_first(uint64_t timeout) { } } + atomic_sub(&queue->len, 1); if (timeout == 0) return true; return !spinning_queue_push(re_enqueue + 1); } diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index ffbf86afc..a333c3ae9 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -4314,6 +4314,7 @@ cc_binary( deps = select_gtest() + [ "//test/util:capability_util", "//test/util:multiprocess_util", + "//test/util:save_util", "//test/util:test_main", "//test/util:test_util", "//test/util:thread_util", diff --git a/test/syscalls/linux/processes.cc b/test/syscalls/linux/processes.cc index c4473cf3a..a2d755c26 100644 --- a/test/syscalls/linux/processes.cc +++ b/test/syscalls/linux/processes.cc @@ -19,7 +19,9 @@ #include #include +#include +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/flags/flag.h" #include "absl/strings/str_format.h" @@ -28,6 +30,7 @@ #include "absl/time/time.h" #include "test/util/capability_util.h" #include "test/util/multiprocess_util.h" +#include "test/util/save_util.h" #include "test/util/test_util.h" #include "test/util/thread_util.h" @@ -142,6 +145,29 @@ TEST(Processes, TheadSharesSamePID) { EXPECT_EQ(test_pid, pid_from_child); } +TEST(Processes, ThousandsOfThreads) { + const DisableSave ds; // Too many syscalls. + const int kThreadCount = 1000; + const int kSyscallsPerThread = 1000; + std::unique_ptr threads[kThreadCount]; + int pipe_fds[2]; + ASSERT_THAT(pipe(pipe_fds), SyscallSucceeds()); + + for (int i = 0; i < kThreadCount; i++) { + threads[i] = std::make_unique([&pipe_fds]() { + char c; + EXPECT_THAT(read(pipe_fds[0], &c, 1), SyscallSucceedsWithValue(0)); + for (int j = 0; j < kSyscallsPerThread; j++) { + syscall(SYS_close, -1); + } + }); + } + close(pipe_fds[1]); + for (int i = 0; i < kThreadCount; i++) { + threads[i]->Join(); + } +} + // ExecSwapResult is used to carry PIDs and TIDs in ExecSwapThreadGroupLeader. struct ExecSwapResult { int pipe_fd; // FD to write result data to. @@ -181,8 +207,8 @@ struct ExecSwapArg { }; // ExecSwapPostExec is the third part of the ExecSwapThreadGroupLeader test. -// It is called after the test has fork()'d, clone()'d, and exec()'d into this. -// It writes all the PIDs and TIDs from each part of the test to a pipe. +// It is called after the test has fork()'d, clone()'d, and exec()'d into +// this. It writes all the PIDs and TIDs from each part of the test to a pipe. int ExecSwapPostExec() { std::cerr << "Test exec'd." << std::endl; pid_t pid; @@ -331,7 +357,8 @@ TEST(Processes, ExecSwapThreadGroupLeader) { EXPECT_NE(test_pid, fork_pid); EXPECT_EQ(fork_pid, result.pre_clone_pid); // Sanity check. - // Before cloning, PID == TID, the child thread is leader of its thread group. + // Before cloning, PID == TID, the child thread is leader of its thread + // group. EXPECT_EQ(result.pre_clone_pid, result.pre_clone_tid); // PID should not change with clone.