1621933 - Perma js/src/jit-test/tests/structured-clone/sab-errMsg.js | /builds/worker/workspace/build/src/js/src/jit-test/tests/structured-clone/sab-errMsg.js:7:13 ReferenceError: SharedArrayBuffer is not defined when Gecko 76 merges to Beta on 2020-04-06

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Narcis Beleuzu [:NarcisB]]

[Tracking Requested - why for this release]:

Central as Beta: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=59d43c58418f7e1c9e2215ae9bc0428992f69681&selectedJob=292812869

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=292812869&repo=try&lineNumber=39846

Comment 1

[Tom Tung [:tt, :ttung]]

I wonder if I need // |jit-test| skip-if: !this.SharedArrayBuffer at the top of the test. I will visit this later today or tomorrow

Comment 2

[Narcis Beleuzu [:NarcisB]]

The bustage is caused by https://hg.mozilla.org/mozilla-central/rev/1313df275548
:lhansen , can you please take a look?

Comment 3

[Lars T Hansen [:lth]]

Tom, the fix you propose will work, but more conventionally we use !sharedMemoryEnabled() instead of !this.sharedArrayBuffer.

Comment 4

[Lars T Hansen [:lth]]

(In reply to Narcis Beleuzu [:NarcisB] from comment #2)

The bustage is caused by https://hg.mozilla.org/mozilla-central/rev/1313df275548
:lhansen , can you please take a look?

Technically that patch uncovered a bug in the test case ;-)

Comment 5

[Tom Tung [:tt, :ttung]]

Hmm, it doesn't look like what I understood after I look into it more. This seems to be Linux only base on https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=59d43c58418f7e1c9e2215ae9bc0428992f69681&selectedJob=292812869 and I couldn't reproduce on my Mac after running " ./mach try release -v 75.0b1 --tasks release-sim --migration central-to-beta --no-push ".

(I saw "ReferenceError: SharedArrayBuffer is not defined" and thus was thinking I forgot to disable the test when we disable the SAB)

I'm going to remove the assignee from myself since this doesn't look like an issue on the test itself. It looks some issues around jit-test and shared array buffer at this moment.

(In reply to Lars T Hansen [:lth] from comment #3)

Tom, the fix you propose will work, but more conventionally we use !sharedMemoryEnabled() instead of !this.sharedArrayBuffer.

If !sharedMemoryEnabled() works the same as !this.sharedArrayBuffer, the test has already had it.
https://searchfox.org/mozilla-central/rev/278046367dab878316f60f0bd7f740cf73f3c447/js/src/jit-test/tests/structured-clone/sab-errMsg.js#1

Lars, I guess I had misled you when you replied the needinfo, so I re-needinfo you for comment 2.

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Remaining tasks got canceled for the push mentioned in comment 5, that's the reason Windows spidermonkey didn't show up as failed there.

Comment 7

[Lars T Hansen [:lth]]

OK, I'll take a look in the morning.

Comment 8

[Lars T Hansen [:lth]]

(In reply to Tom Tung [:tt, :ttung] from comment #5)

If !sharedMemoryEnabled() works the same as !this.sharedArrayBuffer, the test has already had it.

No it doesn't. The test has !this.sharedMemoryEnabled, and that is always false, because sharedMemoryEnabled is a shell function that is always defined, whether SharedArrayBuffer is present or not. You gotta love the implicit boolification of every value in JS... Anyway, with the broken skip-if guard, the test case is always executed whether shared memory is enabled or not.

Comment 9

[Lars T Hansen [:lth]]

Now that I think about it, the reason why my patch causes this may be because it actually disables shared memory in the shell if the configuration is !nightly, a consequence of me struggling with js/moz.configure and the moz.configure linter... Previously, shared memory was enabled in the shell for all configurations. Do we know what is best here?

Comment 10

[Tom Tung [:tt, :ttung]]

(In reply to Lars T Hansen [:lth] from comment #8)

(In reply to Tom Tung [:tt, :ttung] from comment #5)

If !sharedMemoryEnabled() works the same as !this.sharedArrayBuffer, the test has already had it.

No it doesn't. The test has !this.sharedMemoryEnabled, and that is always false, because sharedMemoryEnabled is a shell function that is always defined, whether SharedArrayBuffer is present or not. You gotta love the implicit boolification of every value in JS... Anyway, with the broken skip-if guard, the test case is always executed whether shared memory is enabled or not.

Ah, I see. Then, all cases in https://searchfox.org/mozilla-central/search?q=!this.sharedMemoryEnabled&case=false&regexp=false&path= should be changed...

Comment 11

[Lars T Hansen [:lth]]

Ah, good eye.

Will grab this on monday or so (depending on other workload) if nobody else has grabbed it first.

Comment 12

[Lars T Hansen [:lth]]

Attachment: Bug 1621933 - Fix broken jit-test directives. r?ttung

A few test cases tested for the presence of sharedMemoryEnabled in
the global object. This is nonsensical - it is a testing function
that is always present when jit-tests are executed. The guards should
either not be there, or should invoke sharedMemoryEnabled() to test
for the presence of shared memory functionality.

Comment 13

[Lars T Hansen [:lth]]

I've tested this locally by munging config/milestone.txt to simulate beta, and tests pass now while they fail without the patch, as predicted. Will land the patch.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c620d986a9eb
Fix broken jit-test directives. r=ttung

Comment 15

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/c620d986a9eb

Comment 16

[Cosmin Sabou [:CosminS]]

Verified fix in today's beta-sim: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=294038845&repo=try&lineNumber=39885