Closed Bug 1251179 Opened 4 years ago Closed 4 years ago

mozilla::hal_impl::SetCpuSleepAllowed() crashes if compiled for B2G and there's no /sys/power kernel support

Categories

(Firefox OS Graveyard :: GonkIntegration, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox47 fixed)

RESOLVED FIXED
Tracking Status
firefox47 --- fixed

People

(Reporter: _AtilA_, Assigned: gsvelto)

References

Details

Attachments

(1 file)

While porting RPI2 to b2g, I found that this function crashes when the device is supposed to go to sleep mode. The main difference here is that we are using a kernel without power management subsystem enabled, so there's no /sys/power interface.
Blocks: b2g-rpi2
This is the backtrace of the segfault

Program received signal SIGSEGV, Segmentation fault.
mozilla::OffTheBooksMutex::Lock (this=0x0) at ../../dist/include/mozilla/Mutex.h:69
69	  void Lock() { PR_Lock(mLock); }
(gdb) bt
#0  mozilla::OffTheBooksMutex::Lock (this=0x0) at ../../dist/include/mozilla/Mutex.h:69
#1  0xb3a598f8 in Lock (this=<optimized out>) at ../dist/include/mozilla/Monitor.h:35
#2  MonitorAutoLock (aMonitor=..., this=0xbe88eb94) at ../dist/include/mozilla/Monitor.h:78
#3  mozilla::hal_impl::SetCpuSleepAllowed (aAllowed=<optimized out>) at ../../gecko/hal/gonk/GonkHal.cpp:857
#4  0xb3a56d66 in mozilla::hal::SetCpuSleepAllowed (aAllowed=<optimized out>) at ../../gecko/hal/Hal.cpp:412
#5  0xb42361ee in mozilla::dom::PowerManager::SetCpuSleepAllowed (this=this@entry=0xaf986600, aAllowed=<optimized out>) at /home/jgomez/b2g/build.rpi2.test/gecko/dom/power/PowerManager.cpp:195
#6  0xb3dfc1f6 in mozilla::dom::MozPowerManagerBinding::set_cpuSleepAllowed (cx=<optimized out>, obj=..., self=0xaf986600, args=...) at MozPowerManagerBinding.cpp:470
#7  0xb40723a0 in mozilla::dom::GenericBindingSetter (cx=0xacd5a200, argc=<optimized out>, vp=<optimized out>) at /home/jgomez/b2g/build.rpi2.test/gecko/dom/bindings/BindingUtils.cpp:2683
#8  0xb4ce75d6 in CallJSNative (args=..., native=0xb40722d1 <mozilla::dom::GenericBindingSetter(JSContext*, unsigned int, JS::Value*)>, cx=0xacd5a200) at ../../../gecko/js/src/jscntxtinlines.h:235
#9  js::Invoke (cx=cx@entry=0xacd5a200, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:475
#10 0xb4ce7af0 in js::Invoke (cx=cx@entry=0xacd5a200, thisv=..., fval=..., argc=argc@entry=1, argv=0xbe88ed80, rval=...) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:527
#11 0xb4ce7c10 in js::InvokeSetter (cx=cx@entry=0xacd5a200, thisv=..., fval=..., v=...) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:646
#12 0xb4d238d6 in SetExistingProperty (result=..., shape=..., pobj=..., receiver=..., v=..., id=..., obj=..., cx=0xacd5a200) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/NativeObject.cpp:2352
#13 js::NativeSetProperty (cx=0xacd5a200, obj=..., id=..., value=..., receiver=..., qualified=js::Qualified, result=...) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/NativeObject.cpp:2386
#14 0xb4ce586a in SetProperty (result=..., receiver=..., v=..., id=..., obj=..., cx=<optimized out>) at ../../../gecko/js/src/vm/NativeObject.h:1492
#15 SetPropertyOperation (rval=..., id=..., lval=..., op=<optimized out>, cx=<optimized out>) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:286
#16 Interpret (cx=0xacd5a200, state=...) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:2594
#17 0xb4ce733e in js::RunScript (cx=cx@entry=0xacd5a200, state=...) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:425
#18 0xb4ce74d2 in js::Invoke (cx=cx@entry=0xacd5a200, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:493
#19 0xb4ce7af0 in js::Invoke (cx=cx@entry=0xacd5a200, thisv=..., fval=..., argc=argc@entry=1, argv=argv@entry=0xbe88f510, rval=...) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/vm/Interpreter.cpp:527
#20 0xb4b538fa in js::jit::DoCallFallback (cx=0xacd5a200, frame=0xbe88f560, stub_=0xaa8248b8, argc=1, vp=0xbe88f500, res=...) at /home/jgomez/b2g/build.rpi2.test/gecko/js/src/jit/BaselineIC.cpp:6136
#21 0xb285045c in ?? ()
#22 0xb285045c in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
(gdb)
This is failing when trying to lock |sInternalLockCpuMonitor| which is null. The only way for this to happen is if a previous call to |EnableAlarm()| fails which can happen if /dev/alarm does not exist either or if we cannot set the signal handle for SIGUSR1 (unlikely). My guess is that the latter problem is happening here and it can be verified by checking if the logcat contains this message:

Failed to open alarm device: ...
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Gabriele, I tried the patch and worked fine :) Thanks!!
Comment on attachment 8723549 [details] [diff] [review]
[PATCH] Ensure that the lock protecting the CPU sleep functionality is always valid

This replaces the monitor we previously used to protect accessing the CPU wake lock with a StaticMutex. This has two effects: first of all we guarantee that it's created even when EnableAlarm() fails because /dev/alarm is missing, secondly the mutex is created in a thread-safe way so it doesn't matter which of the functions using it get called first - or if they get called at the same time and race for it.
Attachment #8723549 - Flags: review?(dhylands)
Comment on attachment 8723549 [details] [diff] [review]
[PATCH] Ensure that the lock protecting the CPU sleep functionality is always valid

Review of attachment 8723549 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8723549 - Flags: review?(dhylands) → review+
Thanks for the review, pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/9f716491fee0
https://hg.mozilla.org/mozilla-central/rev/9f716491fee0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.