Skip to content

Rust: Fix, increase worker thread stack size - #1873

Merged
jmillan merged 5 commits into
v3from
stack-overflow-fix
Jul 28, 2026
Merged

Rust: Fix, increase worker thread stack size#1873
jmillan merged 5 commits into
v3from
stack-overflow-fix

Conversation

@jmillan

@jmillan jmillan commented Jul 28, 2026

Copy link
Copy Markdown
Member

The mediasoup worker thread was spawned with the default stack size (macOS secondary thread stack size is ~2060 KB), causing a stack overflow when a WebRtcTransport with SCTP/DataChannels connected.

Association::AssertIsConsistent() calls MS_ASSERT 38 times. Each MS_ASSERT expands to MS_ABORT, which declares a local buffer:

char abortMessage[Logger::BufferSize];  // 50,000 bytes

38 × 50,000 = 1,900,000 bytes ≈ 1808 KB

This was confirmed by runtime probes and disassembly:

sub sp, sp, #0x1c3, lsl #12  ; 1,847,296 bytes
sub sp, sp, #0xf30           ;     3,888 bytes

When triggered from the WebRtcTransport DTLS path, which already has 334 KB of frames on the stack before reaching SCTP, the total exceeds the limit:

334 KB (DTLS base) + 1808 KB (AssertIsConsistent prologue) = 2142 KB
                                                            > 2060 KB → crash

Fix:

Spawn the worker thread with an explicit 8 MB stack via
std::thread::Builder::new().stack_size(8 * 1024 * 1024), giving
6062 KB of headroom at the deepest observed call site.

Also adds Utils::PrintThreadStack() / PrintStackTrace() — a stack-depth measurement utility used to diagnose this issue.

@ibc

ibc commented Jul 28, 2026

Copy link
Copy Markdown
Member

char abortMessage[Logger::BufferSize]; // 50,000 bytes

Perhaps this should be a thread static buffer?

Why isn't this a problem when using mediasoup with Node?

@jmillan

jmillan commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Why isn't this a problem when using mediasoup with Node?

Because the stack size for the process is bigger.

@jmillan

jmillan commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Perhaps this should be a thread static buffer?

Yes, we can do it thread static. I'm making it.

@ibc

ibc commented Jul 28, 2026

Copy link
Copy Markdown
Member

Perhaps this should be a thread static buffer?

Yes, we can do it thread static. I'm making it.

Wait wait, let's please check it carefully. For example, why aren't we using the already existing thread_local char Logger::buffer[Logger::BufferSize]? See Logger.cpp and Logger.hpp.

The mediasoup worker thread was spawned with the default stack size
(macOS secondary thread stack size is ~2060 KB), causing a stack overflow
when a WebRtcTransport with SCTP/DataChannels connected.

Association::AssertIsConsistent() calls MS_ASSERT 38 times. Each
MS_ASSERT expands to MS_ABORT, which declares a local buffer:

    char abortMessage[Logger::BufferSize];  // 50,000 bytes

    38 × 50,000 = 1,900,000 bytes ≈ 1808 KB

This was confirmed by runtime probes and disassembly:

    sub sp, sp, #0x1c3, lsl #12  ; 1,847,296 bytes
    sub sp, sp, #0xf30           ;     3,888 bytes

When triggered from the WebRtcTransport DTLS path, which already
has 334 KB of frames on the stack before reaching SCTP, the total
exceeds the limit:

    334 KB (DTLS base) + 1808 KB (AssertIsConsistent prologue) = 2142 KB
                                                                > 2060 KB → crash

Fix:

  Spawn the worker thread with an explicit 8 MB stack via
  std::thread::Builder::new().stack_size(8 * 1024 * 1024), giving
  6062 KB of headroom at the deepest observed call site.

Also adds Utils::PrintThreadStack() / PrintStackTrace() — a stack-depth
measurement utility used to diagnose this issue.
@jmillan
jmillan force-pushed the stack-overflow-fix branch from 4101adc to 91f7550 Compare July 28, 2026 14:06
@jmillan

jmillan commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Wait wait, let's please check it carefully. For example, why aren't we using the already existing thread_local char Logger::buffer[Logger::BufferSize]?

that's what I've done exactly. check last commit.

@ibc

ibc commented Jul 28, 2026

Copy link
Copy Markdown
Member

IMHO this is the only change we need:

diff --git a/worker/include/Logger.hpp b/worker/include/Logger.hpp
index 24092d64f..63f2fa810 100644
--- a/worker/include/Logger.hpp
+++ b/worker/include/Logger.hpp
@@ -470,11 +470,10 @@ public:
 #define MS_ABORT(desc, ...) \
 	do \
 	{ \
-		std::fprintf(stderr, "(ABORT) " _MS_LOG_STR_DESC desc _MS_LOG_SEPARATOR_CHAR_STD, _MS_LOG_ARG, ##__VA_ARGS__); \
+		std::snprintf(Logger::buffer, Logger::BufferSize, "(ABORT) " _MS_LOG_STR_DESC desc _MS_LOG_SEPARATOR_CHAR_STD, _MS_LOG_ARG, ##__VA_ARGS__); \
+		std::fprintf(stderr, "%s", Logger::buffer); \
 		std::fflush(stderr); \
-		char abortMessage[Logger::BufferSize]; \
-		std::snprintf(abortMessage, Logger::BufferSize, "(ABORT) " _MS_LOG_STR_DESC desc _MS_LOG_SEPARATOR_CHAR_STD, _MS_LOG_ARG, ##__VA_ARGS__); \
-		throw std::runtime_error(abortMessage); \
+		throw std::runtime_error(Logger::buffer); \
 	} \
 	while (false)
 #endif

@jmillan

jmillan commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Changeing CHANGELOG's

Comment thread CHANGELOG.md Outdated
Comment thread rust/CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
@jmillan
jmillan merged commit 9e964ec into v3 Jul 28, 2026
58 checks passed
@jmillan
jmillan deleted the stack-overflow-fix branch July 28, 2026 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants