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

fix for re-request expired resources causing infinite loop #4255

Merged
merged 2 commits into from
Feb 10, 2017

Conversation

tobinbradley
Copy link
Contributor

@tobinbradley tobinbradley commented Feb 10, 2017

Credit to @AndyMoreland for figuring the problem out (see discussion on #3944).

setTimeout has a maximum value of 2147483647 (32bit integer) for all browsers. If set higher than that, the timeout is immediately fired.

For a tile with an expiry longer than ~24 days, like a tile with a max-age=2592000 Cache-Control header (30 days), tileExpires - new Date().getTime() > 2147483647, and you end up in an infinite tile fetch loop.

Fortunately the browser knows the tile hasn't expired so it feeds GL JS from the browser cache and doesn't pound the tile server. But it does pound the browser pretty hard.

setTimeout has a maximum value of 2147483647 (32bit integer) for all browsers. If set higher than that. the timeout is immediately fired. For tile cache expiry longer than 24 days, this results in an infinite tile fetch loop.
Copy link
Contributor

@lbud lbud left a comment

Choose a reason for hiding this comment

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

Minor issue regarding the readability of that number but otherwise this is great, thanks @tobinbradley (and @AndyMoreland)!

@@ -438,7 +438,7 @@ class SourceCache extends Evented {
this._timers[id] = setTimeout(() => {
this.reloadTile(id, 'expired');
this._timers[id] = undefined;
}, tileExpires - new Date().getTime());
}, Math.min(tileExpires - new Date().getTime(), 2147483647));
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this to Math.pow(2, 31) - 1 (for clarity)? Same below.

@tobinbradley
Copy link
Contributor Author

Done. Thanks! That is more clear than a random wildly-large number.

@lbud lbud merged commit 2ab51ec into mapbox:master Feb 10, 2017
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