1604190 - mozmill debug builds crashing | application crashed [@ js::AddTypePropertyId(JSContext*, JSObject*, JS::PropertyKey, JS::Value const&)]

Status: RESOLVED FIXED

(Thunderbird :: Testing Infrastructure, defect)

Description

[Treeherder Bug Filer]

Filed by: mkmelin+mozilla [at] iki.fi
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=281359546&repo=comm-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/MsV5OAu3TFiNif3Lno7OwQ/runs/0/artifacts/public/logs/live_backing.log

---

Comment 1

[Magnus Melin [:mkmelin]]

Looks like something in/with momill causes all the debug builds to fail (seen since builds got going again today).
If it's something mozmill specific, maybe it's time to consider pulling the plug from that...

Comment 2

[Jorg K (CEST = GMT+2)]

Maybe a good idea to investigate before sweeping it under the carpet.
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ca62389e0be395eaba2e7a4c698f78e6a0&tochange=0016ade97e65a470bc97521ab8338210e7
Your crash comes from here:
https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/js/src/vm/TypeInference-inl.h#669
https://searchfox.org/mozilla-central/diff/cecc3baf0ead1b073b9733ee22eb634a4dfdad27/js/src/vm/TypeInference-inl.h#668
which was added here https://hg.mozilla.org/mozilla-central/rev/a49a644cc220#l5.186 in bug 1603373 part 4.

Comment 3

[Jan de Mooij [:jandem]]

I think the bug is in XPConnect. Here the component loader gets a property from the lexical environment:

https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/js/xpconnect/loader/mozJSComponentLoader.cpp#1220

What's likely happening is that this property is the "uninitialized lexical" magic value. This means it's a let/const that hasn't been properly initialized yet.

To debug which property this is, I'd suggest changing this line from:

    if (!JS_SetPropertyById(cx, aExports, symbolId, value)) {

to:

    if (value.isMagic() || !JS_SetPropertyById(cx, aExports, symbolId, value)) {

That should report a JS exception with the name of the property and hopefully that's enough to point you to the code.

I'll fix this properly in XPConnect but that will likely also end up reporting an exception.

Comment 4

[Jan de Mooij [:jandem]]

If this still doesn't help, please ping me on IRC/Slack.

Comment 5

[ISHIKAWA, Chiaki]

Thanks. I will try and report the result here ASAP. I am editing and compiling now.

Comment 6

[ISHIKAWA, Chiaki]

OK, with the change mentioned in comment 3, my local |make mozmill| using the DEBUG version of TB proceeds.

Interesting message I see now instead of the Assertion is as follows:

console.log: === JS Bridge: Starting server
console.log: === Mozmill: Seen window chrome://messenger/content/messenger.xul
JavaScript error: resource://testing-common/mozmill/mozmill.jsm, line 69: Error: resource://testing-common/mozmill/init.jsm - Could not get symbol 'mozmill'.

Now, you say it is possible that there is an uninitialized data.
I can run valgrind version (that takes time).
Although the whole process takes a long time, I think I can get to the
initial path where the uninitialized data can be accessed and let valgrind print warning.
Will that help?

Comment 7

[Jan de Mooij [:jandem]]

Valgrind won't help with this because it's a different kind of 'uninitialized'. It's caused by mozmill.jsm importing init.jsm, but init.jsm then imports mozmill.jsm so you get a cycle and the "mozmill" const in init.jsm becomes the special uninitialized-lexical value.

I posted a patch in bug 1604429, it should fix this.

Comment 8

[Jan de Mooij [:jandem]]

A temporary workaround could be to change const mozmill to var mozmill in init.jsm. There could be similar issues elsewhere, though.

Comment 10

[ISHIKAWA, Chiaki]

An interim report.

I have not applied the patch in 1604429 or did swap |const| and |var| in the previous comment yet.
In that state, I notice there are still cases when the TB (as XPCOM server) seems to crash without clear message, but I need to wait for the
full |make mozmill| test to finish before showing the result.

Comment 11

[ISHIKAWA, Chiaki]

In the interim state, i.e. as reported in comment 10, I got the following crashing TB instances for the following tests.
The test name after "Running directory:" with a traceback line is where the TB did not start, I think during |make mozmill| test.
But it is probably due to the nature of uninitialized status.

...
INFO | (runtestlist.py) | Running directory: instrumentation
Traceback (most recent call last):
INFO | (runtestlist.py) | Running directory: junk-commands
INFO | (runtestlist.py) | Running directory: keyboard
Traceback (most recent call last):
INFO | (runtestlist.py) | Running directory: message-header
INFO | (runtestlist.py) | Running directory: message-reader
INFO | (runtestlist.py) | Running directory: message-window
Traceback (most recent call last):
INFO | (runtestlist.py) | Running directory: multiple-identities
INFO | (runtestlist.py) | Running directory: newmailaccount
INFO | (runtestlist.py) | Running directory: notification
INFO | (runtestlist.py) | Running directory: override-main-menu-collapse
Traceback (most recent call last):
INFO | (runtestlist.py) | Running directory: pref-window
Traceback (most recent call last):
INFO | (runtestlist.py) | Running directory: quick-filter-bar
INFO | (runtestlist.py) | Running directory: search-window
INFO | (runtestlist.py) | Running directory: session-store
INFO | (runtestlist.py) | Running directory: smime
Traceback (most recent call last):
INFO | (runtestlist.py) | Running directory: startup-firstrun
Traceback (most recent call last):
INFO | (runtestlist.py) | Running directory: subscribe

I will report back later on the use of 1604429.

Comment 12

[ISHIKAWA, Chiaki]

With the patch in 1604429 (and reverting the change in comment 3) gets the local |make mozmill| with debug version of TB proceeding.
I don't see the strange error previously seen any more.
Strange error I saw was:

JavaScript error: resource://testing-common/mozmill/mozmill.jsm, line 69: Error: resource://testing-common/mozmill/init.jsm - Could not get symbol 'mozmill'.

But the whole local run has just started. I will report back about an hour later when the test finishes.
Stay tuned.

Comment 13

[Magnus Melin [:mkmelin]]

Attachment: bug1604190_mozmill_fix.patch

The whole mozmill.jsm and init.jsm inclusion seems ... very unnecessary.

Comment 14

[ISHIKAWA, Chiaki]

Without the patch in comment 13 above, my local |make mozmill| run with DEBUG version of TB (with the patch in bug 1604429) ran more or less successfully, and I see the following at the end. Not perfect, but something we can work with. I mean there are usual bunch of bugs, but not the bustage as before.

SUMMARY-PASS | test-iteratorUtils.js::test_toArray_custom_content_iterator
SUMMARY-PASS | test-iteratorUtils.js::teardownModule
INFO | (runtestlist.py) | utils: 9 passed, 0 failed
INFO | (runtestlist.py) | Directories Run: 33, Passed: 1001, Failed: 5
make: *** [/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/mail/testsuite-targets.mk:33: mozmill] Error 1

At one time, I saw the "Passed:" more than 1100. So we still need to improve and enable skipped tests gradually, I suppose.

Comment 15

[Magnus Melin [:mkmelin]]

Comment on attachment 9116371 [details] [diff] [review]
bug1604190_mozmill_fix.patch

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

Looks like the mozmill tests are happy with this. I'll be landing it later as bustage fix. https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=49ec9d993f638d2491f09bd1f34494a472ef2dc9

Comment 16

[Jorg K (CEST = GMT+2)]

Yay for not sweeping it under the carpet as first suggested.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e83883bc47e7
don't needlessly cross-import between the mozmill.jsm and init.jsm. r=bustage-fix

Comment 18

[ISHIKAWA, Chiaki]

(In reply to Magnus Melin [:mkmelin] from comment #15)

Comment on attachment 9116371 [details] [diff] [review]
bug1604190_mozmill_fix.patch

Review of attachment 9116371 [details] [diff] [review]:

Looks like the mozmill tests are happy with this. I'll be landing it later
as bustage fix.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-
central&revision=49ec9d993f638d2491f09bd1f34494a472ef2dc9

Magnus, you don't need my review, do you?
(Anyway, I applied your patch locally, and there does not seem to be any ill-effect, etc. |make mozmill| is running smoothly.)
So it is OK as it is. If that is what you need, yes I am happy with it. r+=chiaki or whatever.

Comment 19

[ISHIKAWA, Chiaki]

Comment on attachment 9116371 [details] [diff] [review]
bug1604190_mozmill_fix.patch

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

This looks reasonable.
I applied the patch locally and ran |make mozmill| with the full debug version of TB and the test proceeded smoothly without introducing any additional bugs.

So I think we should take it.

Comment 20

[ISHIKAWA, Chiaki]

I am not familiar with the recent review board UI or whatever is called. But now I realize how the review process proceeds to produce a template-based output as in comment 19.

Comment 21

[Magnus Melin [:mkmelin]]

(In reply to ISHIKAWA, Chiaki from comment #18)

Magnus, you don't need my review, do you?

I landed it before review to get out of the busted state for debug builds (rs=bustage-fix), but it's still good to get a second pair of eyes on it, so thanks for the review!

Comment 22

[ISHIKAWA, Chiaki]

(In reply to Magnus Melin [:mkmelin] from comment #21)

(In reply to ISHIKAWA, Chiaki from comment #18)

Magnus, you don't need my review, do you?

I landed it before review to get out of the busted state for debug builds (rs=bustage-fix), but it's still good to get a second pair of eyes on it, so thanks for the review!

You are welcome and I am happy that we can build TB again and with better M-C infrastructure, this time loader improvement.

Comment 23

[Intermittent Failures Robot]

33 failures in 4582 pushes (0.007 failures/push) were associated with this bug in the last 7 days.

This is the #49 most frequent failure this week.

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **

Repository breakdown:

• comm-central: 33

Platform breakdown:

• linux32: 9
• macosx1014-64: 8
• linux64: 8
• windows7-32: 4
• windows10-64: 4

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1604190&startday=2019-12-16&endday=2019-12-22&tree=all