As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 547712 - Remove scatter from js shell
: Remove scatter from js shell
Status: RESOLVED DUPLICATE of bug 650411
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: All All
: -- normal (vote)
: ---
Assigned To: general
: Jason Orendorff [:jorendorff]
: 624099 (view as bug list)
Depends on: 545962
  Show dependency treegraph
Reported: 2010-02-22 06:08 PST by Jason Orendorff [:jorendorff]
Modified: 2011-08-12 00:50 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

v1 (52.48 KB, patch)
2010-02-22 18:10 PST, Jason Orendorff [:jorendorff]
jimb: review+
Details | Diff | Splinter Review
Disable remaining scatter-based test. (624 bytes, patch)
2011-01-10 12:39 PST, Jim Blandy :jimb
jorendorff: review+
Details | Diff | Splinter Review

Description User image Jason Orendorff [:jorendorff] 2010-02-22 06:08:43 PST
It has been pretty broken since shavarrays. Shell workers (bug 545962) are the new plan.
Comment 1 User image Jason Orendorff [:jorendorff] 2010-02-22 08:17:17 PST
Several of the tests are worth carrying forward from scatter() to shell workers. I've done the translations, but those that need gc() or some other shell function don't work yet, because workers get a crippled global environment. So I'll have to fix that first.
Comment 2 User image Jason Orendorff [:jorendorff] 2010-02-22 18:10:38 PST
Created attachment 428347 [details] [diff] [review]
Comment 3 User image Igor Bukanov 2010-02-22 23:04:23 PST
(In reply to comment #0)
> It has been pretty broken since shavarrays. 

It was unsafe since e4x appearance in SM. But that does not mean that a safe use does not exist. To make scatter safe one just need need to avoid unsafe features. For this reason most if not all scatter tests are safe. Moreover, the scatter is invaluable currently for the test coverage of bugs related to the GC and to models WebWorkers in the absence of shell workers.

Thus we should consider removal of scatter only when we would get workers working in the shell.
Comment 4 User image Jason Orendorff [:jorendorff] 2010-02-23 07:46:39 PST
Right. Only two of the scatter tests are really broken. dekker.js never terminates under the JIT; for-in.js crashes because it shares arrays across threads.

This patch is intended to go in after the patch in bug 545962. It replaces the scatter tests with Worker tests where possible.

A few of the scatter tests actually test contention on objects shared across threads. The patch just deletes those.
Comment 5 User image Jason Orendorff [:jorendorff] 2010-02-23 08:14:06 PST
The reason I see scatter as "broken" is that I don't really understand the thread safety bugs we have well enough to tell the safe uses from unsafe ones.

For example, I said for-in.js crashes because it shares arrays. I really don't know, though.  The dangers with arrays, I think, are that resizing can race with dslots accesses, and slowification can race with dense array operations. for-in.js doesn't resize or slowify as far as I can see, but it does sporadically crash in array_enumerate.
Comment 6 User image Igor Bukanov 2010-03-01 12:24:06 PST
I am still reviewing the patch. My current worry is that using the Worker.scripts hack would not allow for the tests to run in a browser. On the other hand splitting the tests in many small files is very inconvenient. 

AFAICS there are two workarounds. First we can try to support the data url for workers. This way one can use new Worker("data:,"+encodeURLPart(worker_source)) to run inlined worker_source. But a quick test shows that FF does not support data url for workers...

If that is indeed true, then we can create a single eval worker source like:

  onmessage = function(e) { eval(; }

That should cover most test cases.
Comment 7 User image Jeff Walden [:Waldo] (remove +bmo to email) 2010-03-01 16:06:36 PST
Nothing in the workers spec suggests that data: URLs wouldn't be valid workers.  Bug, or misdiagnosis?
Comment 8 User image Igor Bukanov 2010-03-01 17:45:25 PST
For a test like:

function f() {
   var w = new Worker("data:,"+encodeURIComponent("1;"));
   w.onmessage = function(arg) { alert(arg); }

<body onload="f()">

I am getting in the error console:

Error: Failed to load script: data:,1%3B (nsresult = 0x805303f4)
Comment 9 User image Jesse Ruderman 2010-07-23 07:19:58 PDT
fwiw, all the tests using scatter() were disabled in bug 572600.
Comment 10 User image Jim Blandy :jimb 2011-01-10 12:09:07 PST
*** Bug 624099 has been marked as a duplicate of this bug. ***
Comment 11 User image Jim Blandy :jimb 2011-01-10 12:39:58 PST
Created attachment 502575 [details] [diff] [review]
Disable remaining scatter-based test.

This is less drastic, and avoids intermittent failures until we are ready to clean house.
Comment 12 User image Jim Blandy :jimb 2011-01-10 14:31:36 PST
[bug should remain open]
Comment 13 User image Luke Wagner [:luke] 2011-08-12 00:50:30 PDT

*** This bug has been marked as a duplicate of bug 650411 ***

Note You need to log in before you can comment on or make changes to this bug.