Skip to content

Do Agent Cluster requirements have TC39 consensus? #886

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
annevk opened this issue Apr 12, 2017 · 16 comments
Closed

Do Agent Cluster requirements have TC39 consensus? #886

annevk opened this issue Apr 12, 2017 · 16 comments

Comments

@annevk
Copy link
Member

annevk commented Apr 12, 2017

If an agent is terminated not by programmatic action of its own or of another agent in the cluster but by forces external to the cluster, then the embedding must choose one of two strategies: Either terminate all the agents in the cluster, or provide reliable APIs that allow the agents in the cluster to coordinate so that at least one remaining member of the cluster will be able to detect the termination, with the termination data containing enough information to identify the agent that was terminated.

This allows for two strategies, but since HTML does not have APIs that tell you when something was forcibly closed (and I very much doubt we'd ever get agreement on that), I tried to enforce the former via this PR: whatwg/html#1457. In particularly, that PR removes the ability for user agents to just drop a worker at random and instead have the worker lifetime always be tied to its owner(s).

That PR however led to objections from WebKit relayed by @beidson. @domenic suggested I file an issue on this since he wasn't sure this was actually discussed in a meeting.

@lars-t-hansen
Copy link
Contributor

This was discussed in meetings and there have been issues pertaining to it (this one is still open: whatwg/html#1004), but since it's mostly a consideration for embedders it may not have gotten all the attention it deserves.

Note that without the mechanism mandated by the quoted clause, deadlocks will result immediately when an agent holding a lock is gunned down in its critical section without notification by the embedding but others are allowed to live. Since "lock" and "critical section" are ideas resulting from usage of atomic operations, and not primitive concepts that can be tracked by an implementation, there's basically no way for the embedding to know whether an agent is holding a lock. cc @erights

@domenic
Copy link
Member

domenic commented Apr 12, 2017

Yes, I think it's critical we understand whether this requirement actually got consensus from WebKit folks (cc @beidson, @msaboff) and if it did not, it should get reverted like any other requirement which failed the consensus process.

That may have bad consequences for deadlocks or something, but that's fine, compared to having spec fiction that did not gain consensus and will not be implemented. So far indeed it seems like only Firefox has explicitly agreed to implement this understanding the implications.

@lars-t-hansen
Copy link
Contributor

lars-t-hansen commented Apr 18, 2017

That may have bad consequences for deadlocks or something, but that's fine, compared to having spec fiction

A meaning of the word "fine" with which I was not previously familiar.

Random worker killings are already a problem for web programming as explained in links hanging off of tc39/proposal-ecmascript-sharedmem#27 (comment) and SAB just makes the pain more acute.

that did not gain consensus and will not be implemented.

It's fascinating to me that we could have been going through more than 10 drafts of this spec over two years and not have the representatives for the browser vendors, who show up in generous numbers, manage to object to this earlier.

(Edited for grammar.)

@annevk
Copy link
Member Author

annevk commented Apr 18, 2017

It's actually fairly normal for things to go unnoticed for years right up until the point they get tested or pointed out explicitly. It's rather frustrating, but par for the course when editing standards.

@domenic
Copy link
Member

domenic commented Apr 18, 2017

In this case it's particularly bad because this slipped through the new TC39 process, which requires two implementations before being merged into the specification. As such at the May TC39 meeting my plan is to get a concrete yes/no from the committee, with a default of reverting (as a matter of process) unless we have explicit yes.

@lars-t-hansen
Copy link
Contributor

You probably need to kill the forward progress guarantee while you're at it, since I doubt it means much if an agent can be killed by the embedding without notification.

In fact, the idea of an agent cluster seems to be at risk - the agents in the cluster can block waiting for shared-memory communication, but that communication can be prevented by the UA by killing the sending agent or otherwise interfering with it, so there's no integrity in the cluster at all.

cc @syg @jfbastien @erights @binji

@domenic
Copy link
Member

domenic commented Apr 18, 2017

Yes, if we don't have agreement from all engines to implement the semantics of agent clusters, they should probably be removed or significantly revised. I am pretty shocked we merged this PR at this point.

@annevk
Copy link
Member Author

annevk commented Apr 18, 2017

Well, let's not get ahead of ourselves, we haven't actually heard back from Apple and their previous objection was from June 2016. They might have changed their mind.

@hober
Copy link

hober commented Apr 18, 2017

So long as shared workers and service workers aren't in the same agent cluster as the main page and its dedicated workers, I'm comfortable with the semantics quoted by @annevk at the top of this issue. See also.

@annevk
Copy link
Member Author

annevk commented Apr 18, 2017

@hober thanks for clarifying. I think we're all good here then!

@syg
Copy link
Contributor

syg commented Apr 18, 2017

@domenic My memory of the consensus is of when @lars-t-hansen presented the "all agents must be killed in the cluster" semantics at the 2016-07 meeting at MSFT. @erights gave two thumbs up and nobody objected.

@lars-t-hansen
Copy link
Contributor

@syg's comment made me think about the presentation slides for the various meetings where these semantics might have been discussed:

Substantial fraction of the March 2016 agents spec slides, notably slides titled "Agent Clusters" and "Suspension and Termination":
https://github.com/tc39/ecmascript_sharedmem/blob/master/tc39/agents-mar-2016.pdf

Page 18 of my March 2016 SAB meeting slides:
https://github.com/tc39/ecmascript_sharedmem/blob/master/tc39/sharedmem-mar-2016.pdf

Page 4 of my July 2016 SAB meeting slides (last bullet):
https://github.com/tc39/ecmascript_sharedmem/blob/master/tc39/sharedmem-jul-2016.pdf

I know none of these prove consensus was reached, but there was ample space for dissent at the March meeting and the fact that dissent is not represented in the July slides suggests there was none.

@annevk
Copy link
Member Author

annevk commented Apr 19, 2017

I suspect that @domenic would be okay with closing this issue given @hober's comment, but I'll let him make that decision. I'm certainly fine with closing it at this point, as the interaction with navigation/history is something I consider out-of-scope and is already tracked sufficiently elsewhere.

@littledan
Copy link
Member

cc @nhiroki

@domenic
Copy link
Member

domenic commented Apr 19, 2017

Yeah, we should be good to go now. Sorry for escalating the tensions a bit; it really pushes my buttons when someone suggests keeping things in the spec because they're in the spec, despite implementer pushback. Thankfully the implementer pushback here was not as relevant as we suspected.

@domenic domenic closed this as completed Apr 19, 2017
@lars-t-hansen
Copy link
Contributor

Just realized that this old gem ("Allow script aborting?") is somewhat relevant: #401. The issue is whether the slow-script-killer and possibly other uncatchable exceptions should result in a notification to the page, so that it knows that a computation was aborted and the state of the system may be unpredictable.

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

No branches or pull requests

6 participants