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

Globals have their own microtask queue that is separate from script thread's queue #20908

Open
jdm opened this issue Jun 3, 2018 · 11 comments
Open
Labels
A-content/script Related to the script thread E-candidate-for-mentoring I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring.

Comments

@jdm
Copy link
Member

jdm commented Jun 3, 2018

GlobalScope has a microtask queue that is used to support promises in worker runtimes. This is used from places like AutoEntryScript, which directly invokes GlobalScope::perform_a_microtask_checkpoint. Meanwhile, ScriptThread has its own perform_a_microtask_checkpoint which ignores the microtasks of any globals associated with that thread and uses its own internal microtask queue, which is manipulated by code like MutationObserver. We need to move the queue out of GlobalScope and into worker-specific subtypes, and ensure that GlobalScope delegates to ScriptThread or the subtypes as necessary.

@jdm jdm added the A-content/script Related to the script thread label Jun 3, 2018
@gterzian
Copy link
Member

gterzian commented Jun 6, 2019

As part of that refactoring, we could maybe add a perform_a_microtask_checkpoint method to

pub trait WorkerEventLoopMethods {

Or it could be a method on WorkerGlobalScope, which by the way is owned by each worker sub-type, so the underlying microtask queue could be owned by either.

@jdm jdm added the I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring. label Jun 6, 2019
@jdm
Copy link
Member Author

jdm commented Jun 6, 2019

Huh, I wonder if this is related to the surprising behaviour that @KiChjang saw in #21456.

@gterzian
Copy link
Member

gterzian commented Jun 6, 2019

Huh, I wonder if this is related to the surprising behaviour that @KiChjang saw in #21456.

Since the parser calls the microtask checkpoints of globals, and there can be several globals in a script-thread, yes I think it could lead to problems. Also the spec mentions "Each event loop has a microtask queue", so we defenitely only want one per script-thread(and per worker thread).

@KiChjang
Copy link
Contributor

KiChjang commented Jul 5, 2019

This sounds like something I should look into.

@KiChjang KiChjang self-assigned this Jul 5, 2019
@KiChjang
Copy link
Contributor

KiChjang commented Jul 5, 2019

@jdm I'm actually quite puzzled when you say that ScriptThread and GlobalScope use their own internal microtask queue, because the queue is currently stored as an Rc<MicrotaskQueue> everywhere and they are essentially shared between the script thread and the global scope.

@jdm
Copy link
Member Author

jdm commented Jul 6, 2019

I agree. I think I must have misread the code, since f903474 is quite a bit older than this issue and adds pretty clear documentation.

However, reading through the code, I think I do see a problem - the checkpoint code relies on the provided set of targets to decide whether to run promise job microtasks or silently discard them. When ScriptThread::perform_a_microtask is executed, the full set of same-origin globals are available. However, if the Window global's microtask check point is invoked instead, all promise reactions will end up using that global as the target, regardless of the pipeline id associated with that job.

To avoid this, I think we should make GlobalScope's microtask checkpoint API delegate to the ScriptThread one if we are a Window global.

@humancalico
Copy link
Contributor

@jdm Can I work on this?

@jdm
Copy link
Member Author

jdm commented Mar 26, 2020

Of course!

@humancalico
Copy link
Contributor

@jdm What do mean when you say if we are a window global and how would I check that?

@jdm
Copy link
Member Author

jdm commented Apr 15, 2020

@humancalico This is an example of some code that casts a GlobalScope into a Window. You can also use some_global.is::<Window>() for a simple yes/no if that is all that's necessary.

@jdm
Copy link
Member Author

jdm commented Apr 15, 2020

More specifically, GlobalScope is a base class of various kinds of globals - Window, WorkerGlobalScope, WorkletGlobalScope, and others. These correspond to the global object of various JS environments - page window objects, dedicated workers, and worklets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-content/script Related to the script thread E-candidate-for-mentoring I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring.
Projects
None yet
Development

No branches or pull requests

4 participants