Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Enforce consistent stack size for Flutter threads #49111

Merged
merged 2 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 71 additions & 2 deletions fml/thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,71 @@

namespace fml {

typedef std::function<void()> ThreadFunction;

class ThreadHandle {
public:
explicit ThreadHandle(ThreadFunction&& function);
~ThreadHandle();

void Join();

private:
#if defined(FML_OS_WIN)
HANDLE thread_;
#else
pthread_t thread_;
#endif
};

#if defined(FML_OS_WIN)
ThreadHandle::ThreadHandle(ThreadFunction&& function) {
thread_ = (HANDLE*)_beginthreadex(
nullptr, Thread::GetDefaultStackSize(),
[](void* arg) -> unsigned {
std::unique_ptr<ThreadFunction> function(
reinterpret_cast<ThreadFunction*>(arg));
(*function)();
return 0;
},
new ThreadFunction(std::move(function)), 0, nullptr);
FML_CHECK(thread_ != nullptr);
}

void ThreadHandle::Join() {
WaitForSingleObjectEx(thread_, INFINITE, FALSE);
}

ThreadHandle::~ThreadHandle() {
CloseHandle(thread_);
}
#else
ThreadHandle::ThreadHandle(ThreadFunction&& function) {
pthread_attr_t attr;
pthread_attr_init(&attr);
int result = pthread_attr_setstacksize(&attr, Thread::GetDefaultStackSize());
FML_CHECK(result == 0);
result = pthread_create(
&thread_, &attr,
[](void* arg) -> void* {
std::unique_ptr<ThreadFunction> function(
reinterpret_cast<ThreadFunction*>(arg));
(*function)();
return nullptr;
},
new ThreadFunction(std::move(function)));
FML_CHECK(result == 0);
result = pthread_attr_destroy(&attr);
FML_CHECK(result == 0);
}

void ThreadHandle::Join() {
pthread_join(thread_, nullptr);
}

ThreadHandle::~ThreadHandle() {}
#endif

#if defined(FML_OS_WIN)
// The information on how to set the thread name comes from
// a MSDN article: http://msdn2.microsoft.com/en-us/library/xcb2z8hs.aspx
Expand Down Expand Up @@ -75,7 +140,7 @@ Thread::Thread(const ThreadConfigSetter& setter, const ThreadConfig& config)
fml::AutoResetWaitableEvent latch;
fml::RefPtr<fml::TaskRunner> runner;

thread_ = std::make_unique<std::thread>(
thread_ = std::make_unique<ThreadHandle>(
[&latch, &runner, setter, config]() -> void {
setter(config);
fml::MessageLoop::EnsureInitializedForCurrentThread();
Expand All @@ -102,7 +167,11 @@ void Thread::Join() {
}
joined_ = true;
task_runner_->PostTask([]() { MessageLoop::GetCurrent().Terminate(); });
thread_->join();
thread_->Join();
}

size_t Thread::GetDefaultStackSize() {
return 1024 * 1024 * 2;
}

} // namespace fml
7 changes: 5 additions & 2 deletions fml/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
#include <functional>
#include <memory>
#include <string>
#include <thread>

#include "flutter/fml/macros.h"
#include "flutter/fml/task_runner.h"

namespace fml {

class ThreadHandle;

class Thread {
public:
/// Valid values for priority of Thread.
Expand Down Expand Up @@ -59,8 +60,10 @@ class Thread {

static void SetCurrentThreadName(const ThreadConfig& config);

static size_t GetDefaultStackSize();

private:
std::unique_ptr<std::thread> thread_;
std::unique_ptr<ThreadHandle> thread_;

fml::RefPtr<fml::TaskRunner> task_runner_;

Expand Down
60 changes: 50 additions & 10 deletions fml/thread_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/fml/build_config.h"
#include "flutter/fml/thread.h"

#if defined(FML_OS_MACOSX) || defined(FML_OS_LINUX) || defined(FML_OS_ANDROID)
Expand All @@ -15,6 +16,11 @@
#else
#endif

#if defined(FML_OS_WIN)
#include <windows.h>
#endif

#include <algorithm>
#include <memory>
#include "gtest/gtest.h"

Expand All @@ -37,6 +43,35 @@ TEST(Thread, HasARunningMessageLoop) {
ASSERT_TRUE(done);
}

TEST(Thread, HasExpectedStackSize) {
size_t stack_size = 0;
fml::Thread thread;

thread.GetTaskRunner()->PostTask([&stack_size]() {
#if defined(FML_OS_WIN)
ULONG_PTR low_limit;
ULONG_PTR high_limit;
GetCurrentThreadStackLimits(&low_limit, &high_limit);
stack_size = high_limit - low_limit;
#elif defined(FML_OS_MACOSX)
stack_size = pthread_get_stacksize_np(pthread_self());
#else
pthread_attr_t attr;
pthread_getattr_np(pthread_self(), &attr);
pthread_attr_getstacksize(&attr, &stack_size);
pthread_attr_destroy(&attr);
#endif
});
thread.Join();

// Actual stack size will be aligned to page size, this assumes no supported
// platform has a page size larger than 16k. On Linux reducing the default
// stack size (8MB) does not seem to have any effect.
const size_t kPageSize = 16384;
ASSERT_TRUE(stack_size / kPageSize >=
fml::Thread::GetDefaultStackSize() / kPageSize);
}

#if FLUTTER_PTHREAD_SUPPORTED
TEST(Thread, ThreadNameCreatedWithConfig) {
const std::string name = "Thread1";
Expand All @@ -45,15 +80,20 @@ TEST(Thread, ThreadNameCreatedWithConfig) {
bool done = false;
thread.GetTaskRunner()->PostTask([&done, &name]() {
done = true;
char thread_name[8];
char thread_name[16];
pthread_t current_thread = pthread_self();
pthread_getname_np(current_thread, thread_name, 8);
pthread_getname_np(current_thread, thread_name, 16);
ASSERT_EQ(thread_name, name);
});
thread.Join();
ASSERT_TRUE(done);
}

static int clamp_priority(int priority, int policy) {
return std::clamp(priority, sched_get_priority_min(policy),
sched_get_priority_max(policy));
}

static void MockThreadConfigSetter(const fml::Thread::ThreadConfig& config) {
// set thread name
fml::Thread::SetCurrentThreadName(config);
Expand All @@ -63,10 +103,10 @@ static void MockThreadConfigSetter(const fml::Thread::ThreadConfig& config) {
int policy = SCHED_OTHER;
switch (config.priority) {
case fml::Thread::ThreadPriority::kDisplay:
param.sched_priority = 10;
param.sched_priority = clamp_priority(10, policy);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the value perhaps be sched_get_priority_min and sched_get_priority_max? The values are platform specific (i.e. on macOS the allowed range is 15-47 for SCHED_OTHER, on Linux it is 0-0 for SCHED_OTHER and for other policies it requires root).

I don't quite understand how these tests were passing at all on linux.

Copy link
Member Author

@knopp knopp Dec 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, actually, I know how. pthread tests never actually ran at all- because of missing build_config.h include FLUTTER_PTHREAD_SUPPORTED was 0 all the time.

break;
default:
param.sched_priority = 1;
param.sched_priority = clamp_priority(1, policy);
}
pthread_setschedparam(tid, policy, &param);
}
Expand All @@ -84,27 +124,27 @@ TEST(Thread, ThreadPriorityCreatedWithConfig) {
int policy;
thread.GetTaskRunner()->PostTask([&]() {
done = true;
char thread_name[8];
char thread_name[16];
pthread_t current_thread = pthread_self();
pthread_getname_np(current_thread, thread_name, 8);
pthread_getname_np(current_thread, thread_name, 16);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails with error 34 when buffer length is less than 16 bytes.

pthread_getschedparam(current_thread, &policy, &param);
ASSERT_EQ(thread_name, thread1_name);
ASSERT_EQ(policy, SCHED_OTHER);
ASSERT_EQ(param.sched_priority, 1);
ASSERT_EQ(param.sched_priority, clamp_priority(1, policy));
});

fml::Thread thread2(MockThreadConfigSetter,
fml::Thread::ThreadConfig(
thread2_name, fml::Thread::ThreadPriority::kDisplay));
thread2.GetTaskRunner()->PostTask([&]() {
done = true;
char thread_name[8];
char thread_name[16];
pthread_t current_thread = pthread_self();
pthread_getname_np(current_thread, thread_name, 8);
pthread_getname_np(current_thread, thread_name, 16);
pthread_getschedparam(current_thread, &policy, &param);
ASSERT_EQ(thread_name, thread2_name);
ASSERT_EQ(policy, SCHED_OTHER);
ASSERT_EQ(param.sched_priority, 10);
ASSERT_EQ(param.sched_priority, clamp_priority(10, policy));
});
thread.Join();
ASSERT_TRUE(done);
Expand Down
4 changes: 3 additions & 1 deletion shell/profiling/sampling_profiler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/shell/profiling/sampling_profiler.h"
#include <thread>

#include "flutter/fml/message_loop_impl.h"
#include "flutter/fml/thread.h"
#include "flutter/shell/profiling/sampling_profiler.h"
#include "flutter/testing/testing.h"
#include "gmock/gmock.h"

Expand Down