Closed Bug 1717386 Opened 2 years ago Closed 2 years ago

ThreadStackHelper::CollectProfilingStackFrame should not allocate memory


(Core :: XPCOM, defect)




91 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox89 --- unaffected
firefox90 --- wontfix
firefox91 --- fixed


(Reporter: mozbugz, Assigned: emilio)




(Keywords: regression)


(1 file)

As seen in bug 1711769 comment 9:

Thread 9 (Thread 0x7f55d51be700 (LWP 178899)):
#0  __lll_lock_wait (futex=futex@entry=0x7f55e7b00020, private=0) at lowlevellock.c:52
#1  0x00007f55e81ed235 in __GI___pthread_mutex_lock (mutex=0x7f55e7b00020) at ../nptl/pthread_mutex_lock.c:135
#2  0x000055d34db48063 in BaseAllocator::malloc(unsigned long) ()
#3  0x000055d34db44cc0 in malloc ()
#4  0x00007f55e0e6910c in nsTSubstring<char>::SetLength(unsigned int) () at /home/jgraham/local/firefox/
#5  0x00007f55e23358d1 in mozilla::ThreadStackHelper::CollectProfilingStackFrame(js::ProfilingStackFrame const&) () at /home/jgraham/local/firefox/
#6  0x00007f55e2314703 in MergeStacks(unsigned int, bool, RegisteredThread const&, Registers const&, NativeStack const&, ProfilerStackCollector&, JS::ProfilingFrameIterator::Frame*, unsigned int) () at /home/jgraham/local/firefox/
#7  0x00007f55dfa03897 in profiler_suspend_and_sample_thread(int, unsigned int, ProfilerStackCollector&, bool) () at /home/jgraham/local/firefox/
#8  0x00007f55dfaae3f5 in mozilla::ThreadStackHelper::GetStack(mozilla::HangStack&, nsTSubstring<char>&, bool) () at /home/jgraham/local/firefox/
#9  0x00007f55e233473f in mozilla::BackgroundHangManager::MonitorThread(void*) () at /home/jgraham/local/firefox/
#10 0x00007f55e79e4acd in _pt_root () at /home/jgraham/local/firefox/

ThreadStackHelper::CollectProfilingStackFrame is trying to expand a string (I'm not sure which one exactly), and that causes a memory allocation, which is forbidden during profiler_suspend_and_sample_thread, because it freezes a thread, which may leave any held mutex in an unstable state.

NI:Doug, you have worked on this recently, could you please have a look? Happy to help -- in particular, we should probably add ⚠ BIG WARNING COMMENTS ⚠ near profiler_suspend_and_sample_thread and any use of it.

Flags: needinfo?(dothayer)

This is almost surely AssignJSLinearString: from bug 1586921, if the linear string length is bigger than the nsAutoCString capacity.

Which explains why bug 1711769 has only been seen on the parent process.

Flags: needinfo?(florian)
Regressed by: 1586921
Has Regression Range: --- → yes

Set release status flags based on info from the regressing bug 1586921

If we have a function bigger than the inline string capacity then we'd
allocate memory potentially causing deadlocks.

Assignee: nobody → emilio
Flags: needinfo?(florian)
Flags: needinfo?(dothayer)
Pushed by
Don't allocate memory in CollectProfilingStackFrame. r=florian,gerald
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch

I have been testing this with nightly, it's looking good so far.

I'm not 100% certain, but my uptime without crash has been much longer than before the fix.

You need to log in before you can comment on or make changes to this bug.