Closed Bug 551680 Opened 14 years ago Closed 14 years ago

replacing JS_(Suspend|Resume)Request calls with JSAutoSuspendRequest

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: igor, Assigned: igor)

References

Details

Attachments

(1 file, 1 obsolete file)

Currently to implement a conservative stack scanning in the bug 516832 the GC must be able to suspend an arbitrary native thread which brings significant complexity to the implementation and constrains embeddings, since Linux does not have a native suspend support, see the bug 538702. This is necessary to support threads that calls JS_SuspendRequest.

On the other hand if a thread that suspends the request contains a single native frame that both calls JS_SuspendRequest and JS_ResumeRequest, then it is necessary to scan the native stack only up to that frame without the need to suspend the thread. Unfortunately the flexibility that separated JS_SuspendRequest and JS_ResumeRequest API provides means that the functions could be called from rather different native frames. So the idea is to replace calls to JS_SuspendRequest with JSAutoSuspendRequest and add a C API taking a callback like JS_PauseRequest(cx, callback). This will ensure that a frame that both suspends and resume exists.

Clearly, for compatibility with older code that would still call JS_SuspendRequest, the conservative stack scanner would need to suspend the thread. But at least that could be ifdefed.
The patch replaces most explicit JS_SuspendRequest and JS_ResumeRequest calls through out the browser and js shell with JSAutoSuspendRequest constructor/destructor. To support the case of the request transfer from one context to another it adds a new API, JS_TransferRequest(cx, another). That allows to avoid any locks while the transfer.

What the patch has not yet done is to eliminate the calls to XPCJSContextStack::Push. Fixing that would require to lift that call up to a few caller frames. That I would like to do that in a separated patch or bug to minimize the number of changes in this patch.
Attachment #431855 - Flags: review?(mrbkap)
Blocks: 552266
The new version insists that JS_TransferRequest is called for an context in request. It also changes EvalInContext from shell/js.cpp to use the new API for the evalcx context.
Attachment #431855 - Attachment is obsolete: true
Attachment #432454 - Flags: review?(mrbkap)
Attachment #431855 - Flags: review?(mrbkap)
Attachment #432454 - Flags: review?(mrbkap) → review+
http://hg.mozilla.org/mozilla-central/rev/eba4f78cdca4
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: