Skip to content

Commit

Permalink
Increase stack size for worker (runtime) thread
Browse files Browse the repository at this point in the history
Summary:
Running AsyncDebuggerAPITest with ASAN enabled causes a bunch of "call
stack size exceeded" errors. Fixing this by switching to use pthread
and set the stack size of the runtime thread to be the same as the main
test runner thread. The runtime thread can be thought of as
HermesRuntime's main thread and needs more stack space.

```
******************** TEST 'Hermes-Unit :: API/./APITests/AsyncDebuggerAPITest.NoDebuggerEventCallbackTest' FAILED ********************
Note: Google Test filter = AsyncDebuggerAPITest.NoDebuggerEventCallbackTest
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from AsyncDebuggerAPITest
[ RUN      ] AsyncDebuggerAPITest.NoDebuggerEventCallbackTest
libc++abi: terminating due to uncaught exception of type facebook::jsi::JSError: Maximum call stack size exceeded (native stack depth)
RangeError: Maximum call stack size exceeded (native stack depth)
```

Reviewed By: mattbfb

Differential Revision: D53145701

fbshipit-source-id: 378c4ce7580d786522bf63f904f2acf088b169dd
  • Loading branch information
dannysu authored and facebook-github-bot committed Jan 30, 2024
1 parent 79d16ca commit 71037bd
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 2 deletions.
1 change: 1 addition & 0 deletions API/hermes/inspector/chrome/tests/ConnectionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <iostream>
#include <limits>
#include <optional>
#include <thread>

#include <gtest/gtest.h>

Expand Down
36 changes: 35 additions & 1 deletion API/hermes/inspector/chrome/tests/SerialExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,30 @@ namespace hermes {
namespace inspector_modern {
namespace chrome {

SerialExecutor::SerialExecutor() {
SerialExecutor::SerialExecutor(size_t stackSize) {
#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
pthread_attr_t attr;

int ret;
ret = pthread_attr_init(&attr);
if (ret != 0) {
throw std::runtime_error("Failed pthread_attr_init");
}

if (stackSize != 0) {
ret = pthread_attr_setstacksize(&attr, stackSize);
if (ret != 0) {
throw std::runtime_error("Failed pthread_attr_setstacksize");
}
}

ret = pthread_create(&tid_, &attr, SerialExecutor::threadMain, this);
if (ret != 0) {
throw std::runtime_error("Failed pthread_create");
}
#else
workerThread_ = std::thread([this]() { this->run(); });
#endif
}

SerialExecutor::~SerialExecutor() {
Expand All @@ -23,7 +45,11 @@ SerialExecutor::~SerialExecutor() {
shouldStop_ = true;
wakeUpSig_.notify_one();
}
#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
pthread_join(tid_, nullptr);
#else
workerThread_.join();
#endif
}

void SerialExecutor::add(std::function<void()> task) {
Expand Down Expand Up @@ -56,6 +82,14 @@ void SerialExecutor::run() {
}
}

#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
void *SerialExecutor::threadMain(void *p) {
static_cast<SerialExecutor *>(p)->run();
pthread_exit(nullptr);
return nullptr;
}
#endif

} // namespace chrome
} // namespace inspector_modern
} // namespace hermes
Expand Down
15 changes: 14 additions & 1 deletion API/hermes/inspector/chrome/tests/SerialExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
#include <functional>
#include <memory>
#include <mutex>
#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
#include <pthread.h>
#else
#include <thread>
#endif

namespace facebook {
namespace hermes {
Expand All @@ -27,7 +31,11 @@ namespace chrome {
class SerialExecutor {
private:
// The thread on which all work is done.
#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
pthread_t tid_;
#else
std::thread workerThread_;
#endif

// A list of functions to execute on the worker thread.
std::deque<std::function<void()>> tasks_;
Expand All @@ -46,10 +54,15 @@ class SerialExecutor {
/// they are posted. This stops running when shouldStop_ is set to true.
void run();

#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
/// Main function of the new thread.
static void *threadMain(void *p);
#endif

public:
/// Construct a thread which will run for the duration of this object's
/// lifetime.
SerialExecutor();
SerialExecutor(size_t stackSize = 0);

/// Make sure that the spawned thread has terminated. Will block if there is a
/// long-running task currently being executed.
Expand Down
8 changes: 8 additions & 0 deletions unittests/API/AsyncDebuggerAPITest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,15 @@ void AsyncDebuggerAPITest::SetUp() {
runtime_ = facebook::hermes::makeHermesRuntime(builder.build());
asyncDebuggerAPI_ = AsyncDebuggerAPI::create(*runtime_);

#if !defined(_WINDOWS) && !defined(__EMSCRIPTEN__)
// Give the runtime thread the same stack size as the main thread. The runtime
// thread is the main thread of the HermesRuntime.
struct rlimit limit;
getrlimit(RLIMIT_STACK, &limit);
runtimeThread_ = std::make_unique<SerialExecutor>(limit.rlim_cur);
#else
runtimeThread_ = std::make_unique<SerialExecutor>();
#endif

eventCallbackID_ = kInvalidDebuggerEventCallbackID;
}
Expand Down

0 comments on commit 71037bd

Please sign in to comment.