Skip to content
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

Clarify that "this" is not available while in parallel #11129

Merged
merged 9 commits into from
Mar 14, 2025
Merged

Conversation

domfarolino
Copy link
Member

@domfarolino domfarolino commented Mar 12, 2025

This fixes #11110, by clarifying that Web IDL's this value is not available from steps running in parallel, even if those steps were kicked off my a non-parallel algorithm that can access "this".

This PR clarifies this in the parallelism example, prose about how to operate from while parallel, and fixes all examples of "this's relevant foo" being used improperly from parallel steps, which is a good start to setting the right precedent.


/canvas.html ( diff )
/infrastructure.html ( diff )
/webappapis.html ( diff )
/worklets.html ( diff )

@domfarolino domfarolino requested a review from annevk March 12, 2025 15:30
@annevk
Copy link
Member

annevk commented Mar 12, 2025

This seems reasonable, but we should fix at least the case I introduced as part of this I think. And maybe also the cases @domenic knows about. People typically copypasta their way into a specification after all.

@annevk annevk requested a review from domenic March 12, 2025 17:07
@domfarolino
Copy link
Member Author

domfarolino commented Mar 12, 2025

I scanned all references of "this's relevant" in the spec and fixed any that were operating incorrectly from parallel steps. That resulted in the offscreen canvas fix, the #parallelism editorial "example" fix, as well as one more instance I could find, in addModule(). I've updated the OP accordingly. I think that's a good start to steering people in the right example without contradicting ourselves too much.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! A couple minor suggestions, but I'm happy to see this cleaned up.

data-x="global object">global</span>, or <span>environment settings object</span>. By extension,
you cannot access Web IDL's <span>this</span> value from steps running <span>in parallel</span>,
even if those steps were activated by an algorithm that <em>does</em> have access to the
<span>this</span> value. (Stated in more familiar terms, you must not directly access main-thread
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new sentence would probably be best as a new paragraph. The parenthetical and the sentence about data races seem a bit awkward after the new interruption.

Copy link
Member Author

@domfarolino domfarolino Mar 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I guess one interesting thing is that this paragraph says we cannot use "global" objects while in parallel, while we clearly agree that we actually make an exception for globals, since that's the only way we can schedule things back on the main thread. In other words, we are assuming #6475 (comment) to be true—that most accesses to a global, especially when scheduling tasks, are atomic. That's the right assumption make IMO. I'm just trying to think if we should state this assumption/exception clearly in this paragraph too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a case to be made for porting (some much shorter version of) w3c/ServiceWorker#1755 (comment) into the spec, to acknowledge the messy reality and carve out some clearer exceptions. But that's a more ambitious followup, I think.

@domenic domenic added clarification Standard could be clearer topic: event loop labels Mar 14, 2025
@domenic domenic merged commit 52c3ca8 into main Mar 14, 2025
2 checks passed
@domenic domenic deleted the in-parallel-this branch March 14, 2025 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer topic: event loop
Development

Successfully merging this pull request may close these issues.

Make it explicit that "this" is available in parallel
3 participants