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

Spec behavior for terminated agents #24

Open
inexorabletash opened this issue Dec 21, 2017 · 6 comments
Open

Spec behavior for terminated agents #24

inexorabletash opened this issue Dec 21, 2017 · 6 comments

Comments

@inexorabletash
Copy link
Member

Two cases need to be specified:

  • When an agent terminates, any associated lock requests need to be dequeued (and the queue processed)
  • When an agent terminates, any associated held locks need to be released (and the queue processed)

I don't know how to specify the "when an agent terminates" part. The association will need to be made explicit, but that part is simple.

@domenic
Copy link

domenic commented Jan 2, 2018

Based on https://tc39.github.io/ecma262/#sec-agent-clusters agent termination appears to be a concept that exists, although not exactly precisely defined. Perhaps we just want to add a <dfn> around "terminate" there and reference it from the predicate.

It might also be worth thinking about agent deactivation/activation (more vague concepts mentioned in that section) and suspension/waiting via Atomics.wait. Maybe no special reactions need to happen, but I thought it'd be worth checking.

@inexorabletash
Copy link
Member Author

Cool, thanks for the pointers @domenic !

@annevk
Copy link
Member

annevk commented Jan 3, 2018

I don't think agent termination is observable currently and we've explicitly decided against APIs exposing it for workers in the past (GC exposure concerns or some such). E.g., when you navigate a document it becomes inactive, but it might not necessarily be terminated. It's quite a bit more complex and if we actually want to expose anything there we'd need to do more digging.

@inexorabletash
Copy link
Member Author

ISTM it's implicitly exposed in a few cases already:

  • Indexed DB - connections held in terminated agents are closed, which can unblock upgrades - observable via events
  • WebSockets - presumably these close in a deterministic/interoperable fashion - not observable by other script, but should be specified?
  • Service Worker - list of clients - observable via polling?

But yeah, we need to be explicit about our intent and spec appropriately if if we move forward here.

@asutherland
Copy link

For ServiceWorker integration, I think it might make sense to invoke the agent integration logic that aborts all requests and releases all held locks if the Service Worker Has No Pending Events algorithm returns true at the same place the ServiceWorker spec invokes Try Activate we do in step 5.2 of ExtendableEvent when the last promise resolves.

Said another way, break all locks when the ServiceWorker becomes eligible for termination.

The idea here would be:

  • It helps saves ServiceWorker authors from lock-related bugs because they should trigger reliably when a lock is not aligned properly with use of waitUntil versus appearing rarely in the field.
  • It helps mitigate the issues with agent termination termination being observable by transforming the implicit into explicit.
    • Assume a keep-alive idiom where a page that really wants to keep its ServiceWorker alive uses a lock to notify it when the ServiceWorker has terminated so it can generate a new event that will restart the ServiceWorker.
      • Strictly speaking, there isn't really a problem with this. After all, the page could just as easily spam overlapping postMessage() calls with timer-based waitUntil calls, or just directly use a Worker or SharedWorker whose lifetimes are tied to the page. But those are explicit costs that are surfaced to heuristics that might decide which tab/window to freeze/terminate. Whereas a ServiceWorker with no pending events might be thought of as a cost borne by the browser in the interest of performance and not directly charged against the page/origin. And arguably it's better for the browser to actually keep the ServiceWorker alive than for it to be continually restarted with all the script re-evaluation overhead, etc. Especially if the termination was tied to resource pressure in the first place.
    • The greater concern is really side-channel leaks, though.

@inexorabletash
Copy link
Member Author

Sounds reasonable to me at first glance.

Want to toss up a PR?

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

No branches or pull requests

4 participants