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

Should terminating a worker close/error streams? #1256

Open
saschanaz opened this issue Feb 17, 2023 · 3 comments
Open

Should terminating a worker close/error streams? #1256

saschanaz opened this issue Feb 17, 2023 · 3 comments

Comments

@saschanaz
Copy link
Member

I found that Firefox currently explicitly closes new Response(...).body when the owning worker closes: https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/dom/base/BodyStream.cpp#169

But for general streams AFAICT both Firefox and Chrome do not close nor error the stream, and just let the read requests stall forever when trying to read streams transferred from terminated workers.

But should worker termination ping the streams what's happening? This will expose worker GC, but Web Locks already exposes that information.

cc @asutherland

@domenic
Copy link
Member

domenic commented Feb 18, 2023

I don't think streams, which are a basic data structure-ish primitive, should have hooks into environment-specific termination signals like this.

If specific streams, e.g. response body streams, want to add such hooks, that's reasonable. (But, their spec should be updated to include that.)

@saschanaz
Copy link
Member Author

saschanaz commented Feb 18, 2023

While that sounds reasonable, wouldn't it be a footgun for other Stream-using specs when the core Streams spec doesn't cover it? File System and WebTransport may also need to deal with this themselves for example.

(Depends on whether we should do this at all, though.)

@domenic
Copy link
Member

domenic commented Feb 21, 2023

Hmm, fair. If this is something we should do (I am still not clear), then it should be as part of the other-spec wrapper algorithms, IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants