Skip to content

Need to define the current Realm for pipeTo operations #845

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

Closed
bzbarsky opened this issue Oct 19, 2017 · 2 comments
Closed

Need to define the current Realm for pipeTo operations #845

bzbarsky opened this issue Oct 19, 2017 · 2 comments
Assignees
Labels

Comments

@bzbarsky
Copy link

https://streams.spec.whatwg.org/#rs-pipe-to defines things like calling ReadableStreamReaderGenericRelease during shutdown. ReadableStreamReaderGenericRelease step 4 needs to create a "promise rejected with", which means it needs to know what global to use for that.

Presumably the global is that of the current Realm. But it's not clear to me that the current Realm is well-defined on all the paths that can lead to shutdown in the parallel section of pipeTo. In particular, nothing really indicates that if dest.[[state]] is or becomes "errored" has to happen while there is a current Realm set up.

It would be good to make this clearer, ideally by explicitly saying what Realm should be used here or having a note explaining how the existence of a current Realm is guaranteed.

We should probably also have tests for this, if we don't already.

@domenic domenic self-assigned this Oct 20, 2017
@domenic domenic added the piping label Mar 7, 2018
@domenic
Copy link
Member

domenic commented Mar 8, 2018

Sorry for the delay here. I think the intent is that we should be queuing a task back to the main thread once we get into any of shutdown with an action or shutdownEDIT: upon further consideration, finalize is the right place for this, as that's the part that starts being observable to user code. So, we should definitely add "queue a task" in there.

Of course, just queuing a task doesn't really help re-establish the current realm. This is reminiscent of our discussions in whatwg/webidl#135, and gives me a new appreciation for @jyasskin's proposal. If we were to do what I was suggesting, and require being explicit, we'd have to pass a realm from pipeTo to ReadableStreamReaderGenericRelease to "a promise resolved with". That is not fun.

On the other hand, I don't think this promise is observable. The reader in question is never exposed to author code, and so you can't observe its [[closedPromise]]. As such, I can't imagine a way to write tests for this. So maybe the right fix for this particular case is to make that clearer.

@bzbarsky
Copy link
Author

bzbarsky commented Mar 8, 2018

On the other hand, I don't think this promise is observable.

Hmm. Claims like that in JS need proof, and the proof is liable to become invalid as TC-39 makes changes....

But yes, if we can make it clear that this thing is not observable that would solve this particular problem.

domenic added a commit that referenced this issue Mar 9, 2018
domenic added a commit that referenced this issue Mar 9, 2018
Closes #845. Although, in the process of writing this we discovered #905. This commit also modifies the spec to highlight that issue, in the nearby text.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants