Enforce consistent stack size for Flutter threads

Author: knopp

fml/thread.cc

@@ -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
@@ -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();
@@ -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

fml/thread.h

@@ -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.
@@ -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_;
 

fml/thread_unittests.cc

@@ -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)
@@ -15,6 +16,10 @@
 #else
 #endif
 
+#if defined(FML_OS_WIN)
+#include <windows.h>
+#endif
+
 #include <memory>
 #include "gtest/gtest.h"
 
@@ -37,6 +42,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";
@@ -45,15 +79,27 @@ 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) {
+  int min = sched_get_priority_min(policy);
+  int max = sched_get_priority_max(policy);
+  if (priority < min) {
+    return min;
+  } else if (priority > max) {
+    return max;
+  } else {
+    return priority;
+  }
+}
+
 static void MockThreadConfigSetter(const fml::Thread::ThreadConfig& config) {
   // set thread name
   fml::Thread::SetCurrentThreadName(config);
@@ -63,10 +109,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);
       break;
     default:
-      param.sched_priority = 1;
+      param.sched_priority = clamp_priority(1, policy);
   }
   pthread_setschedparam(tid, policy, &param);
 }
@@ -84,27 +130,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);
     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);

shell/profiling/sampling_profiler_unittest.cc

@@ -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"