-
Notifications
You must be signed in to change notification settings - Fork 165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
State that by default all objects are created in the relevant realm of this
#135
Comments
This looks like a duplicate of https://www.w3.org/Bugs/Public/show_bug.cgi?id=24652. |
See also web-platform-tests/wpt#1381 for tests which I'm about to land. |
Can we provide a default for in parallel blocks created from IDL methods and constructors too? We'd want to capture the relevant or current realm, respectively, at the point where the in parallel block starts, and use it as the "object-creation realm" within the block. Tasks posted from a context with an "object-creation realm" could inherit the same realm. We still have some cases without an "object-creation realm", like https://webbluetoothcg.github.io/web-bluetooth/#notification-events. These tasks actually have an obvious realm to use, but it's lost when we pull the event loop out of the realm's settings object. We could extend "queue a task" to take its object-creation realm in order to have a default here too. |
That seems not great to me. When you're in a background thread, you should be very explicit about what realm you're creating objects in. Just inheriting a default is not going to work out well in general, I think. IMO it's better to just state the realm when creating the object, and rely on ambient defaults as little as possible---only for the most obvious of cases. We should not be trying to create more situations that have ambient defaults available in non-obvious ways. |
Given:
Do you have any cases in mind where you'd want |x| and |y| to have different globals? |
Yes, definitely. For example if |y| was an event that was going to be dispatched to a different global. The various postMessage specs do this, for example. |
Cool, thanks for the example. https://html.spec.whatwg.org/multipage/comms.html#dom-window-postmessage creates the events inside a task, rather than in parallel, and it'd be straightforward to reset the default when queuing a task... My main concern is that verbosity and repetition hurt readability. If we insist that every object creation in parallel code specify a realm, and that realm is the same for most calls, it makes it harder to follow the rest of the algorithm and obscures the few cases where the realm is importantly different. Are we differing in how often we expect parallel code to need to change the realm? Most of the new specs I'm seeing do a lot of work in parallel but don't shift realms much if ever. When you and @bzbarsky go through existing specs to specify their new objects' realms, do you tend to find that you'd want them to shift realms when they go parallel? |
In general, staying in the same realm when you go parallel makes sense. The main footguns with object creation after going parallel are:
Apart from that, using the same global unless explicitly stated otherwise makes sense to me. |
I don't understand why you'd ever create an object in parallel. Object creation should always happen in an environment with a global. You can schedule a task that ends up creating the object. Otherwise you're inventing new JavaScript semantics and we've been told off for that many times. (FWIW, I think "in parallel" is a bit of an anti-pattern as it often doesn't deal well with races or the constraints of that parallel place. Having explicit task-based isolated-threads would be much clearer.) |
I mean, in practice scheduling a task is the right thing to do, and might happen implicitly (e.g. if you do your work off a promise getting resolved). But you then need to define which global to use anyway.... |
It'd be fine with me if we wrote somewhere that objects cannot be created from parallel contexts, and I agree with @annevk's goal to replace "in parallel" with better-specified threads. I tentatively propose the following resolution to this issue:
|
I still strongly object to trying to make object creation realm implicit. I think specs should use an explicit realm whenever possible. Ideally I would prefer not even having the exceptions outlined in the OP, but they are there as a compromise. This is something spec authors should be aware of and making conscious decisions on. I'd like to get us to a state where things are done explicitly in a widespread way. If we find this leads to a lot of repetition then maybe we can consider setting some defaults. But my suspicion is that it's generally not going to be obvious which realm to create things in when operating in parallel/inside a task. I'd like us to try this before considering a radical step such as implicit propagation of object creation realms. |
I think the object creation realm is one of the things that spec authors are just not going to consider, and so if we don't default it, we'll just wind up with underspecification. However, I'm happy to take the consensus subset for now. Does everyone like the idea to define the object-creation realm and "new object" term, and have "in parallel" and "queue a task" kill off the default? I can't send a PR until next week, but I'll try to do it then. |
I think that's why we should formalize object creation. So that it's an easy-to-review thing in standards and something implementers can more easily flag. Perhaps even lintable some day. The main problem at the moment is the lack of a formalized approach (for many things) where everyone just copy-and-pastes things that mostly work if you squint hard enough without really thinking through the implications (and without being able to think them through since we don't even say what "new" means or link it). |
Agreed with at least formalizing object creation - we've got a lot of object creation in the Typed OM spec, for instance, and didn't realize that we had to specify the global for each. (BZ is flagging every instance of it for us. ^_^) |
Hmm, good question, I'm not sure... I note that the spec for createElement doesn't actually state a realm? Although it has some pretty close ties to the relevant realm via the custom element registry, so maybe it should use relevant? |
Thanks for the reply. I see. I continue to watch the discussion, and I'm happy to follow the conclusion once experts make it. |
So here is my take on this.
I realize this is not what the ES spec does, not least because ES has no concept of "relevant global". ES doesn't even do this for the "create and cache" case, which is pretty weird. Given that, I really don't think we should aim for aligning with ES here (as in, I consider its behavior to just be wrong in a bunch of cases). |
#135 (comment) is pretty convincing to me. I don't really understand the intuition for using the relevant realm, even for lazily created objects. If you were writing this code in JavaScript in the normal way, closing over the class whose instance you're creating, you'd end up with an instance in the current realm (even if this comes through a callback/after an await). |
Oh, I think I understand, if it's lazily cached, it'd be weird if the realm were chosen just based on who randomly asked for it first. |
Note that these semantics differ from JavaScript async functions, which create a Promise in the current Realm. Verified in Chromium and Firefox that HTMLImageElement.decode and CustomElementsRegistry.whenDefined return Promise instances from the relevant realm. (I'm not sure what's the preferred way to typeset "this", or if it would be preferred to thread it explicitly through the algorithm. Also, it's not quite necessary to use the relevant Promise.resolve function, since resolve effectively uses the relevant realm of its receiver(!)) Resolves part of whatwg#135
Note that these semantics differ from JavaScript async functions, which create a Promise in the current Realm. Verified in Chromium and Firefox that HTMLImageElement.decode and CustomElementsRegistry.whenDefined return Promise instances from the relevant realm. (I'm not sure what's the preferred way to typeset "this", or if it would be preferred to thread it explicitly through the algorithm. Also, it's not quite necessary to use the relevant Promise.resolve function, since resolve effectively uses the relevant realm of its receiver(!)) Resolves part of whatwg#135
Another issue is static methods. At least with Chrome and Firefox and at least with // $0 is a same-origin synchronously-accessible iframe
rect = $0.contentWindow.DOMRect.fromRect.call(DOMRect);
rect.constructor === $0.contentWindow.DOMRect // true Firefox also has this weird bug where |
Per current Web IDL spec, static methods aren't even passed a "this" at the post-IDL-spec level, so don't have a concept of "relevant global" or "relevant realm". |
That's quite fair, and I would be satisfied if this is spec'd as such. However, newer ES primitives like |
We did have the ambition to support subclassing of platform objects and make that work nicely in a way that's compatible with JavaScript, but that kind of wavered and in general it does not seem like subclassing has taken of. |
We could change static methods to pass along "this", sure. I'm just saying that right now they do not. |
@annevk tried tackling this again recently. Some discussion in https://freenode.logbot.info/whatwg/20210203#c6775366 . Here's one proposal, motivated largely by the fact that we still haven't solved this. (So, my previous appetite for making things explicit is dampened.)
(*): what does "the current realm" and "the relevant realm" mean when you're inside a queued task? We want to formalize its intuitive meaning: whatever the current/relevant realm was when the original main-thread task kicked off. Proposal:
|
Does that include promises that are then cached? |
Yeah, although I'm not sure how many of those there are (if any)? I think the concern there is not very large, compared to the cost of inconsistency if we continue using current for so many other promises. (Notably, all implementations today use current at least for for synchronously rejected promises, and probably more promises than that to the extent bindings code is involved.) |
This is most critically a concern for Promise-valued attributes (e.g. |
This has come up recently in w3c/webcrypto#85 and also in some internal work Blink is doing on refactoring their bindings layer.
It appears that in all browsers, document.createElement, innerHTML, and an Event created through button.click() are created in the relevant realm of
this
. All of these cases are currently unspecified and use spec text equivalent to "create a new X object".We should specify a few things:
this
(At first I thought that we should follow the ES spec and use the current realm for all cases. That would better match e.g.
Array.prototype.map
orPromise.prototype.then
. But it seems like that's not what browsers do, so we'll just have to live with the inconsistency.)The text was updated successfully, but these errors were encountered: