Hit MOZ_CRASH(mozilla::detail::MutexImpl::lock: pthread_mutex_lock failed) at mozglue/misc/Mutex_posix.cpp:75 with evalInWorker and off-thread scripts

RESOLVED FIXED in Firefox 55

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: decoder, Assigned: sfink)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
mozilla56
x86_64
Linux
assertion, crash, jsbugmon, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 fixed, firefox56 fixed)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 months ago
The following testcase crashes on mozilla-central revision 9851fcb0bf4d (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

evalInWorker(`
  var lfGlobal = newGlobal();
  lfGlobal.offThreadCompileScript(\`{ let x; throw 42; }\`);
`);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff60fe700 (LWP 8726)]
0x00000000004759bf in mozilla::detail::MutexImpl::lock (this=this@entry=0x7ffff69c4e80) at mozglue/misc/Mutex_posix.cpp:74
#0  0x00000000004759bf in mozilla::detail::MutexImpl::lock (this=this@entry=0x7ffff69c4e80) at mozglue/misc/Mutex_posix.cpp:74
#1  0x0000000000a5d1af in js::Mutex::lock (this=this@entry=0x7ffff69c4e80) at js/src/threading/Mutex.cpp:58
#2  0x000000000044cf62 in js::LockGuard<js::Mutex>::LockGuard (aLock=..., this=<synthetic pointer>) at js/src/threading/LockGuard.h:25
#3  js::AutoLockMonitor::AutoLockMonitor (monitor=..., this=<synthetic pointer>) at js/src/vm/Monitor.h:47
#4  js::shell::OffThreadState::markDone (this=0x7ffff69c4e80, newToken=0x7ffff4213d50) at js/src/shell/js.cpp:208
#5  0x0000000000b4f240 in js::HelperThread::handleParseWorkload (this=this@entry=0x7ffff694ec00, locked=...) at js/src/vm/HelperThreads.cpp:1717
#6  0x0000000000b51418 in js::HelperThread::threadLoop (this=this@entry=0x7ffff694ec00) at js/src/vm/HelperThreads.cpp:1975
#7  0x0000000000b514a0 in js::HelperThread::ThreadMain (arg=0x7ffff694ec00) at js/src/vm/HelperThreads.cpp:1490
#8  0x0000000000b59602 in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::callMain<0ul> (this=0x7ffff691e0a0) at js/src/threading/Thread.h:234
#9  js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start (aPack=0x7ffff691e0a0) at js/src/threading/Thread.h:227
#10 0x00007ffff7bc16fa in start_thread (arg=0x7ffff60fe700) at pthread_create.c:333
#11 0x00007ffff6c38b5d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
rax	0x0	0
rbx	0x16	22
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7ffff60fcef0	140737321619184
rsp	0x7ffff60fcee0	140737321619168
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff60fe700	140737321625344
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff69c4e80	140737330826880
r13	0x7ffff60fcfb0	140737321619376
r14	0x7ffff60fe6f8	140737321625336
r15	0x7ffff60fcfb8	140737321619384
rip	0x4759bf <mozilla::detail::MutexImpl::lock()+63>
=> 0x4759bf <mozilla::detail::MutexImpl::lock()+63>:	movl   $0x0,0x0
   0x4759ca <mozilla::detail::MutexImpl::lock()+74>:	ud2

Updated

5 months ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]

Comment 1

5 months ago
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/95f01f3075fe
user:        Steve Fink
date:        Wed Mar 15 17:03:42 2017 -0700
summary:     Bug 1337209 - Add JS shell test mechanism for gray marking, r=jonco

This iteration took 264.300 seconds to run.
Steve, is bug 1337209 a likely regressor?
Flags: needinfo?(sphink)
(Assignee)

Comment 3

5 months ago
Ugh. Yes. I had to mess around with the shutdown ordering.

I guess I'll need to split things up or something. There appears to be no valid ordering, the way things are now.
(Assignee)

Comment 4

5 months ago
Created attachment 8872775 [details] [diff] [review]
Nest ShellContext lifetime within JSContext

...though I can't get this simple reordering to crash now. And it makes more sense this way.
Attachment #8872775 - Flags: review?(jcoppeard)
(Assignee)

Comment 5

5 months ago
Created attachment 8872776 [details] [diff] [review]
Nest ShellContext lifetime within JSContext

...though I can't get this simple reordering to crash, and it makes more sense.
Attachment #8872776 - Flags: review?(jcoppeard)
(Assignee)

Updated

5 months ago
Attachment #8872775 - Attachment is obsolete: true
Attachment #8872775 - Flags: review?(jcoppeard)
(Assignee)

Updated

5 months ago
Assignee: nobody → sphink
Status: NEW → ASSIGNED

Updated

5 months ago
Attachment #8872776 - Flags: review?(jcoppeard) → review+
Steve, do we want to try to land this before merge day?
Flags: needinfo?(sphink)
(oops...)
Flags: needinfo?(sphink)

Comment 8

4 months ago
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/369de2fe16f5
Nest ShellContext lifetime within JSContext, r=jonco

Comment 9

4 months ago
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4135740caaa4
add regression test for JS shell ordering problem, r=me
(Assignee)

Comment 10

4 months ago
Comment on attachment 8872776 [details] [diff] [review]
Nest ShellContext lifetime within JSContext

Approval Request Comment
[Feature/Bug causing the regression]: bug 1337209
[User impact if declined]: crash in JS shell tests only
[Is this code covered by automated tests?]: yes, but accidentally left it out of this patch. Landed in a separate push: https://hg.mozilla.org/integration/mozilla-inbound/rev/4135740caaa4588b80449a58a5f6582f834f5306
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: JS shell only
[String changes made/needed]: none
Flags: needinfo?(sphink)
Attachment #8872776 - Flags: approval-mozilla-beta?
Backed out the test for SM failures like https://treeherder.mozilla.org/logviewer.html#?job_id=108349942&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/775e7d3da5ec5eb5e76ec4cad6687509be800694

Sfink was going to check to see if the original patch also needs to come out.
Flags: needinfo?(sphink)

Comment 12

4 months ago
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5ac2907c262
add regression test for JS shell ordering problem, r=me
(Assignee)

Comment 13

4 months ago
Re-landed. It turns out that one of the functions in that test will crash when run with --no-threads, so you're supposed to detect that case and skip the test.
Flags: needinfo?(sphink)

Comment 14

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/369de2fe16f5
https://hg.mozilla.org/mozilla-central/rev/b5ac2907c262
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Comment 15

4 months ago
(In reply to Steve Fink [:sfink] [:s:] from comment #13)
> Re-landed. It turns out that one of the functions in that test will crash
> when run with --no-threads, so you're supposed to detect that case and skip
> the test.

As :decoder pointed out, I erroneously called this a "crash". It's just an error message and a failed status code, which apparently is expected.
Blocks: 1337209
status-firefox54: --- → unaffected
status-firefox-esr52: --- → unaffected
Comment on attachment 8872776 [details] [diff] [review]
Nest ShellContext lifetime within JSContext

js shell fix, beta55+
Attachment #8872776 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 17

4 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/9724b598971f
status-firefox55: affected → fixed
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.