On Fri, Apr 09 2021, Mark Kettenis <mark.kette...@xs4all.nl> wrote: >> Date: Fri, 9 Apr 2021 07:09:06 +1000 >> From: Jonathan Matthew <jonat...@d14n.org> >> >> 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. Index: Makefile =================================================================== RCS file: /cvs/ports/www/mozilla-firefox/Makefile,v retrieving revision 1.457 diff -u -p -r1.457 Makefile --- Makefile 19 Apr 2021 08:20:50 -0000 1.457 +++ Makefile 20 Apr 2021 06:28:37 -0000 @@ -6,6 +6,7 @@ ONLY_FOR_ARCHS = amd64 i386 aarch64 # Don't forget to bump www/firefox-i18n after updates. MOZILLA_VERSION = 88.0 +REVISION= 0 MOZILLA_BRANCH = release MOZILLA_PROJECT = firefox MOZILLA_CODENAME = browser Index: patches/patch-third_party_libwebrtc_webrtc_rtc_base_platform_thread_cc =================================================================== RCS file: patches/patch-third_party_libwebrtc_webrtc_rtc_base_platform_thread_cc diff -N patches/patch-third_party_libwebrtc_webrtc_rtc_base_platform_thread_cc --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ patches/patch-third_party_libwebrtc_webrtc_rtc_base_platform_thread_cc 20 Apr 2021 06:28:37 -0000 @@ -0,0 +1,18 @@ +$OpenBSD$ + +Avoid calling pthread_setschedparam() with a possibly uninitialized +thread parameter. The creator thread may set the thread_ member after +the created thread runs. + +Index: third_party/libwebrtc/webrtc/rtc_base/platform_thread.cc +--- third_party/libwebrtc/webrtc/rtc_base/platform_thread.cc.orig ++++ third_party/libwebrtc/webrtc/rtc_base/platform_thread.cc +@@ -360,7 +360,7 @@ bool PlatformThread::SetPriority(ThreadPriority priori + param.sched_priority = top_prio; + break; + } +- return pthread_setschedparam(thread_, policy, ¶m) == 0; ++ return pthread_setschedparam(pthread_self(), policy, ¶m) == 0; + #endif // defined(WEBRTC_WIN) + } + -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
