On Tue, Apr 20, 2021 at 09:06:27AM +0200, Jeremie Courreges-Anglas wrote: > On Fri, Apr 09 2021, Mark Kettenis <[email protected]> wrote: > >> Date: Fri, 9 Apr 2021 07:09:06 +1000 > >> From: Jonathan Matthew <[email protected]> > >> > >> On Thu, Apr 08, 2021 at 10:24:06AM +0200, Martin Pieuchot wrote: > >> > firefox often crash when somebody else connects to the jitsi I'm in. > >> > The trace looks like a stack exhaustion, see below. > >> > > >> > Does this ring a bell? > >> > > >> > #530 <signal handler called> > >> > #531 pthread_setschedparam (thread=0x0, policy=1, param=0x2ade5ca1df0) > >> > at /usr/src/lib/librthread/rthread_sched.c:56 > >> > #532 0x000002ae65f9f016 in > >> > rtc::PlatformThread::SetPriority(rtc::ThreadPriority) () from > >> > /usr/local/lib/firefox/libxul.so.101.0 > >> > #533 0x000002ae65f9ed75 in rtc::PlatformThread::Run() () > >> > from /usr/local/lib/firefox/libxul.so.101.0 > >> > #534 0x000002ae65f9ead9 in rtc::PlatformThread::StartThread(void*) () > >> > from /usr/local/lib/firefox/libxul.so.101.0 > >> > #535 0x000002ade9d4df51 in _rthread_start (v=<optimized out>) > >> > at /usr/src/lib/librthread/rthread.c:96 > >> > #536 0x000002ad9c2ec3da in __tfork_thread () > >> > at /usr/src/lib/libc/arch/amd64/sys/tfork_thread.S:84 > >> > >> This looks like, at least with our pthreads implementation, libwebrtc has a > >> race between the newly created thread and the thread creating it. > >> > >> The creating thread stores the thread handle in thread_ here: > >> https://github.com/mozilla/gecko-dev/blob/master/third_party/libwebrtc/webrtc/rtc_base/platform_thread.cc#L186 > >> > >> and the new thread uses it here: > >> https://github.com/mozilla/gecko-dev/blob/master/third_party/libwebrtc/webrtc/rtc_base/platform_thread.cc#L363 > >> which is called as almost the first thing the new thread does. > >> > >> Our pthread_create() only stores the pthread handle into the supplied > >> address after __tfork_thread() returns, by which time the new thread > >> could already be running. > > > > Right. POSIX isn't very explicit about this, but it does state that > > the thread ID is stored upon successful completion. So I don't think > > portable code can rely on that until pthread_create() has returned. > > > > Using pthread_self() in the pthread_setschedparam() call will probably > > help. > > It does indeed, firefox diff below. > > > But if PlatformThread::GetThreadRef() gets called early on in > > the thread, there will still be a race. Setting thread_ at the start of > > PlatformThread::Run() would be better. > > This brings questions, like how to do the same thing on Windows and > non-Windows. Lots of TODO entries in that code. As far as I'm > concerned I'll leave significant rethinking to upstream.
Fwiw, i'm not qualified to judge anything in this area, but i've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1706261 upstream at mozilla if ppl want to discuss the specificities (or a better fix/refactoring). note that mozilla is a libwebrtc downstream, the 'real' upstream is at https://webrtc.googlesource.com/src/ (and i suppose somehow shared with chrome) but im not going there, my last experience with trying to discuss things with google was a nightmare. Landry
