-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Nit: timer initialization steps should define maximum for id #7358
Comments
Well, what happens if you hit the maximum? The return value is only Looks like Chromium just loops back to 1, or... will infinite-loop, if the map is full?? https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/dom_timer_coordinator.cc;l=46-47;drc=de68be3f18ba99cc01d75903e167ca09bade253c I was also wondering if this should get more specific and specify that it's an incrementing per-global counter, since I'm pretty sure that's what everyone implements. |
Talking about this issue a bit with Chromium folks and they're OK with our current infinite-loop behavior... basically, the reasoning is: timers (including the function or string) occupy at least a few bytes, so you're very likely to run out of memory and crash before hitting 2^31 timers. (E.g., 2^31 * 16 bytes = 34 GiB.) And, we already allow I'm still not super-comfortable with the situation since I don't know what to write in the spec text. Like, the spec does acknowledge resource limitations, but as written this part of the timer initialization steps spec will basically be saying "choose a not-previously-used integer within the range 1 to 2^31" which is clearly impossible after a certain point. This feels different in kind than writing spec text like "increase counter by 1" where it's implicitly understood than in most cases implementations will not use a bigint, and so increasing the counter will start being weird around 2^32 or 2^64. For the record, I spent a bit of time perusing Firefox's code and from what I can tell they just loop back to 1 and don't care about the collision. Maybe the easy path here is just something like
Although we don't like implementation-defined behavior in the general case, in this case it's probably not worth spending time on since it's pretty theoretical. |
Adding @smaug---- and @farre for thoughts from Firefox. I suppose implementation-defined is reasonable for the limit case. At least until it starts happening in the future for one reason or another. It behaving as incrementing per global might be nice to define though. And if that's defined it also makes it possible for developers to bail before they hit the limit. |
After the refactoring this section will say
which is equivalent to what it says today. However, it seems a little bad that you have to infer from IDL what the maximum is here. I suspect to some extent it will depend on whatwg/infra#87. Using typed integers to indicate the range would make the most sense to me as that is what implementations do already.
The text was updated successfully, but these errors were encountered: