Closed Bug 1503968 Opened 6 years ago Closed 6 years ago

abort due too small stack for Watchdog

Categories

(Core :: XPConnect, defect, P2)

Other
Unspecified
defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: dan, Assigned: dan)

References

Details

Attachments

(1 file)

A debug build of FF trunk is crashing with an abort after setting thread stack size on my ppc64le system (Fedora 28), see the backtrace bellow. The problem is that js/xpconnect/src/XPCJSContext.cpp hardcodes the stack size to 32K, but this is too small for ppc64/ppc64le (and aarch64 probably), especially when they run with 64KB page size. Their minimum is 128KB.

Setting kWatchdogStackSize to 0 would make PR_CreateThread() to use a default stack size. I would prefer to avoid arch specific values.


(gdb) where
#0  0x00007fffa3a99d3c in __libc_signal_restore_set (set=0x7fffed08bc88) at ../sysdeps/unix/sysv/linux/nptl-signals.h:80
#1  0x00007fffa3a99d3c in raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:48
#2  0x00007fff98a94bc0 in nsProfileLock::FatalSignalHandler(int, siginfo_t*, void*) (signo=11, info=0x7fffed08cce8, context=0x7fffed08bf70)
    at /mnt/dan/firefox/toolkit/profile/nsProfileLock.cpp:177
#3  0x00007fff998fbfe8 in js::UnixExceptionHandler(int, siginfo_t*, void*) (signum=11, info=0x7fffed08cce8, context=0x7fffed08bf70)
    at /mnt/dan/firefox/js/src/ds/MemoryProtectionExceptionHandler.cpp:288
#4  0x00007fffa3b004d8 in <signal handler called> () at arch/powerpc/kernel/vdso64/sigtramp.S
#5  0x00000001142892a0 in mozalloc_abort(char const*) (msg=0x1142d9340 "Redirecting call to abort() to mozalloc_abort\n") at /mnt/dan/firefox/memory/mozalloc/mozalloc_abort.cpp:35
#6  0x00000001142892ec in abort() () at /mnt/dan/firefox/memory/mozalloc/mozalloc_abort.cpp:82
#7  0x00007fffa34037a8 in PR_Assert (s=0x7fffa3448b80 "0 == rv", file=0x7fffa3448b48 "/mnt/dan/firefox/nsprpub/pr/src/pthreads/ptthread.c", ln=356)
    at /mnt/dan/firefox/nsprpub/pr/src/io/prlog.c:553
#8  0x00007fffa343dde0 in _PR_CreateThread (type=PR_USER_THREAD, start=0x7fff910498b4 <WatchdogMain(void*)>, arg=0x14a55c530, priority=PR_PRIORITY_NORMAL, scope=PR_GLOBAL_THREAD, state=PR_JOINABLE_THREAD, stackSize=32768, isGCAble=0) at /mnt/dan/firefox/nsprpub/pr/src/pthreads/ptthread.c:356
#9  0x00007fffa343e3d0 in PR_CreateThread (type=PR_USER_THREAD, start=0x7fff910498b4 <WatchdogMain(void*)>, arg=0x14a55c530, priority=PR_PRIORITY_NORMAL, scope=PR_GLOBAL_THREAD, state=PR_JOINABLE_THREAD, stackSize=32768) at /mnt/dan/firefox/nsprpub/pr/src/pthreads/ptthread.c:518
#10 0x00007fff9107b8d8 in Watchdog::Init() (this=0x14a55c530) at /mnt/dan/firefox/js/xpconnect/src/XPCJSContext.cpp:151
#11 0x00007fff9107ce58 in WatchdogManager::StartWatchdog() (this=0x14a565590) at /mnt/dan/firefox/js/xpconnect/src/XPCJSContext.cpp:419
#12 0x00007fff9107cbf0 in WatchdogManager::RefreshWatchdog() (this=0x14a565590) at /mnt/dan/firefox/js/xpconnect/src/XPCJSContext.cpp:392
#13 0x00007fff9107c2c4 in WatchdogManager::RegisterContext(XPCJSContext*) (this=0x14a565590, aContext=0x14a561c20) at /mnt/dan/firefox/js/xpconnect/src/XPCJSContext.cpp:300
#14 0x00007fff9104b914 in XPCJSContext::XPCJSContext() (this=0x14a561c20) at /mnt/dan/firefox/js/xpconnect/src/XPCJSContext.cpp:988
#15 0x00007fff9104bf80 in XPCJSContext::NewXPCJSContext(XPCJSContext*) (aPrimaryContext=0x0) at /mnt/dan/firefox/js/xpconnect/src/XPCJSContext.cpp:1246
#16 0x00007fff910e10e0 in nsXPConnect::nsXPConnect() (this=0x14a561bf0) at /mnt/dan/firefox/js/xpconnect/src/nsXPConnect.cpp:76
#17 0x00007fff910e1458 in nsXPConnect::InitStatics() () at /mnt/dan/firefox/js/xpconnect/src/nsXPConnect.cpp:131
#18 0x00007fff91068df8 in xpcModuleCtor() () at /mnt/dan/firefox/js/xpconnect/src/XPCModule.cpp:13
#19 0x00007fff966596ac in nsLayoutModuleInitialize() () at /mnt/dan/firefox/layout/build/nsLayoutModule.cpp:234
#20 0x00007fff8f73f480 in nsComponentManagerImpl::Init() (this=0x14a4e14d0) at /mnt/dan/firefox/xpcom/components/nsComponentManager.cpp:360
#21 0x00007fff8f84bf20 in NS_InitXPCOM2(nsIServiceManager**, nsIFile*, nsIDirectoryServiceProvider*) (aResult=0x14a511d90, 
    aBinDirectory=0x14a457930, aAppFileLocationProvider=0x7fffed08dae8) at /mnt/dan/firefox/xpcom/build/XPCOMInit.cpp:695
#22 0x00007fff98aa8ba0 in ScopedXPCOMStartup::Initialize() (this=0x14a511d90) at /mnt/dan/firefox/toolkit/xre/nsAppRunner.cpp:1483
#23 0x00007fff98ab8330 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=0x7fffed08da68, argc=4, argv=0x7fffed08f1b8, aConfig=...)
    at /mnt/dan/firefox/toolkit/xre/nsAppRunner.cpp:4918
#24 0x00007fff98ab883c in XRE_main(int, char**, mozilla::BootstrapConfig const&) (argc=4, argv=0x7fffed08f1b8, aConfig=...) at /mnt/dan/firefox/toolkit/xre/nsAppRunner.cpp:5014
#25 0x00007fff98ad4a4c in mozilla::BootstrapImpl::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=0x14a422140, argc=4, argv=0x7fffed08f1b8, aConfig=...)
    at /mnt/dan/firefox/toolkit/xre/Bootstrap.cpp:49
#26 0x0000000114287148 in do_main(int, char**, char**) (argc=4, argv=0x7fffed08f1b8, envp=0x7fffed08f1e0) at /mnt/dan/firefox/browser/app/nsBrowserApp.cpp:237
#27 0x0000000114287500 in main(int, char**, char**) (argc=4, argv=0x7fffed08f1b8, envp=0x7fffed08f1e0) at /mnt/dan/firefox/browser/app/nsBrowserApp.cpp:329
Kris, I *think* you changed this in bug 1476405. Can you take a look?
Flags: needinfo?(kmaglione+bmo)
Priority: -- → P2
The watchdog thread stacks need to be as small as possible. If that means we need platform-specific values, then so be it, I guess.

But PPC is a tier 3 platform, and I don't have time to devote to it.
Flags: needinfo?(kmaglione+bmo)
What would you say to following? Or it could be MAX(32K, PTHREAD_STACK_MIN). Or result of sysconf(_SC_THREAD_STACK_MIN).

diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp
index 11171e49f5..bd547dace8 100644
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -81,8 +81,12 @@ using namespace JS;
 using mozilla::dom::AutoEntryScript;
 
 // The watchdog thread loop is pretty trivial, and should not require much stack
-// space to do its job. So only give it 32KiB.
+// space to do its job. So only give it 32KiB or the platform minimum.
+#if defined(PTHREAD_STACK_MIN)
+static constexpr size_t kWatchdogStackSize = PTHREAD_STACK_MIN;
+#else
 static constexpr size_t kWatchdogStackSize = 32 * 1024;
+#endif
 
 static void WatchdogMain(void* arg);
 class Watchdog;
Attachment #9022527 - Flags: review?(bobbyholley) → review+
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/53412b7c8303
set Watchdog thread stack size to at least platform minimal stack size. r=bholley
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/53412b7c8303
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Assignee: nobody → dan
Regressions: 1720740
Regressions: 1721326
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: