Closed Bug 7049 Opened 25 years ago Closed 25 years ago

JS engine must be "runtime pair-wise" thread-safe

Categories

(Core :: JavaScript Engine, defect, P3)

All
Other
defect

Tracking

()

VERIFIED WONTFIX

People

(Reporter: brendan, Assigned: fur)

References

()

Details

(Whiteboard: [Perf])

Dougt@netscape.com is using a separate JS runtime and context(s) in a background thread to avoid locking up the UI with lenght scripts and their native unzip and file i/o operations. This seems to work, but there are several truly global variables (http://lxr.mozilla.org/seamonkey/ident?i=deflated_string_cache being the example I remember) that aren't protected unless JS_THREADSAFE is defined. That means subtle crash-bugs down the road. I don't believe the client needs JS_THREADSAFE to be defined -- LiveConnect and OJI should connect in a way that interlocks JS execution, as the MozillaClassic codebase did (pls. confirm, fur). If we can avoid defining JS_THREADSAFE by protecting the few globals not factored into JSRuntimes, that would be best. A new JS_RUNTIME_THREADSAFE or somesuch macro could be tested. /be
As far as LiveConnect interlocking to prevent more than one thread in the JS engine, I can only guess because that behavior is determined by callbacks defined by OJI code. I assume that this interlocking is done because I've often discussed this issue with Patrick and others.
A couple of months ago, I did an analysis of shared, non-constant globals in the JS binary so as to see if multiple JSRuntimes could be accomodated without JS_THREADSAFE. (Results are below). It would be trivial to protect these few variables using locks without invoking the full horror of JS_THREADSAFE, though the JS engine would now require linking with NSPR. Alternatively, we could do more aggressive changes to move all the dangerous globals into the JSRuntime data structure. Shared globals: jsarena.c: arena_freelist jsstr.c: deflated_string_cache deflated_string_cache_bytes jsnum.c, jsdtoa.c, fdlibm/*.c errno (Isn't thread-safe even when JS_THREADSAFE is defined !) jsdtoa.c: freelist p5s
We can't move the deflated_string_cache variables into a JSRuntime without breaking API compatibility (JS_GetStringBytes takes only JSString* and no JSContext* or JSRuntime*). Adding Patrick just to be sure about the LiveConnect issue, which boils down to running JS called from Java where it can safely execute DOM code related to the page that contains the Java applet or 4.x-style plugin (or that called Java from JS without benefit of any applet/plugin). Until layout is multithreaded, that means running JS called from Java on the ui/layout thread. /be
Ack, it appears JS_THREADSAFE was turned on already. Maybe we don't care about its performance hit? /be
Are you sure that JS_THREADSAFE is turned on for *all* platforms ? I seem to recall that there was some question as to whether this was the case.
Assignee: clayton → fur
To you, fur. This one should have an active owner.
Whiteboard: [Perf]
Putting on [Perf] radar
Component: JavaScript → Javascript Engine
moving to the "JavaScript Engine" component.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → WONTFIX
JS_THREADSAFE is turned on for all platforms, so there is no requirement for the JS-engine to be "runtime pair-wise" thread-safe, except perhaps for performance. If that turns out to be the case, we can reinvestigate this issue later. I'm writing up a separate bug for the non-thread-safe dependency on errno, which is not protected against races even when JS_THREADSAFE is defined.
Target Milestone: M7
Status: RESOLVED → VERIFIED
marking verified to get it off my radar. if any one disagrees they can reopen it.
You need to log in before you can comment on or make changes to this bug.