878850 - Why do the JSContext-taking rooting methods check for being in a request?

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Right now, these two pieces of code:

  JS_AddNamedValueRoot(cx, &foo, "bar");

  JS_AddNamedValueRootRT(JS_GetRuntime(cx), &foo, "bar");

behave differently: the former requires cx to be in a request, while the latter obviously does not.

Why do we need the "in a request" check?

Comment 1

[Bobby Holley (:bholley)]

I think luke has the best idea of why we need to be in a request these days.

Comment 2

[Luke Wagner [:luke]]

This is all historical; no request is needed now.  Requests only serve a few remaining vestigial purposes (such as asking if anything is running).

Comment 3

[Terrence Cole [:terrence]]

I found out recently that we also only do conservative stack scanning if there is an active request.

Comment 4

[Terrence Cole [:terrence]]

To elaborate: we should remove the in-request check from JS_AddNamedValueRoot after we switch to exact rooting.

Comment 5

[Terrence Cole [:terrence]]

Attachment: remove_check_request_in_rooted-v0.diff

The IsInRequest check in Rooted was to ensure we didn't make any stack stores when the conservative scanner was not going to be running. Now that that is all gone, no more need for the check. It looks like there aren't any calls to any of JS_IsInRequest or js::IsInRequest anymore either, so we can kill those too. I think may be one more use of requestDepth still; regardless, we're closing in on killing Requests.

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=77b158741b72

Comment 6

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8538860 [details] [diff] [review]
remove_check_request_in_rooted-v0.diff

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

\o/ yeehaw!

Comment 7

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6a4f6e2043d4

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6a4f6e2043d4