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

Add the resource management specification. #26

Closed
wants to merge 0 commits into from

Conversation

yutakahirano
Copy link
Owner

The user agent can terminate the fetch operation and reclaim associated resources if the response body stream is not reachable and there is not an active reader. #15

@yutakahirano
Copy link
Owner Author

@domenic, can you take a look at the change?

A ReadableByteStream _s_ is said to be __observable__ when one of the following meets:

- _s_ is reachable from the Garbage Collector.
- _s_ has an active reader.
Copy link
Contributor

Choose a reason for hiding this comment

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

, "i.e., IsReadableStreamLocked(s) is true"

Copy link
Contributor

Choose a reason for hiding this comment

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

What if s's reader is not reachable? Hmm...

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, if reader is not reachable, we still have problems with reader.read().then(...) or reader.closed.then(...), right.

Maybe add a non-normative note to the effect: "The intention here is that once the developer acquires a reader for the body stream, they have taken responsibility for resource management into their own hands, and plan to call s.cancel() if they are no longer interested in consuming the body."

Copy link
Owner Author

Choose a reason for hiding this comment

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

"i.e., IsReadableStreamLocked(s) is true"

But it is for ReadableStream, right? Can I use it for ReadableByteStream?

Oh, if reader is not reachable, we still have problems with reader.read().then(...) or reader.closed.then(...), right.
Maybe add a non-normative note to the effect: "The intention here is that once the developer acquires a reader for the body stream, they have taken responsibility for resource management into their own hands, and plan to call s.cancel() if they are no longer interested in consuming the body."

Thanks, I added that.

Copy link
Contributor

Choose a reason for hiding this comment

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

"i.e., IsReadableStreamLocked(s) is true"

But it is for ReadableStream, right? Can I use it for ReadableByteStream?

Siiigh, I guess not, technically :-/. But, IsReadableByteStreamLocked will be added to the spec at the same time ReadableByteStream is. So it depends on your strategy for referencing ReadableByteStream.

It is probably fine to leave as-is.

Copy link
Owner Author

Choose a reason for hiding this comment

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

OK, done. I will fix the link later.

@domenic
Copy link
Contributor

domenic commented Mar 18, 2015

(review done :))

@domenic
Copy link
Contributor

domenic commented Mar 18, 2015

I would add this example:

// The user agent must not terminate the communication because there is an
// active reader. Even though the reader is not reachable by the garbage collector,
// the reader has been used to make termination observable, so if termination
// occurred on GC, that would make GC observable.
fetch("http://www.example.com/").then(res => {
  res.body.getReader().closed.then(() => console.log("stream closed!"));
});

@yutakahirano
Copy link
Owner Author

I would add this example:

Done.

@yutakahirano yutakahirano deleted the resource-management branch March 30, 2015 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants