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

prevent infinite loop #3887

Merged
merged 1 commit into from
Nov 10, 2019
Merged

Conversation

tanhauhau
Copy link
Member

@tanhauhau tanhauhau commented Nov 10, 2019

@tanhauhau tanhauhau changed the title WIP: prevent infinite loop prevent infinite loop Nov 10, 2019
@Rich-Harris Rich-Harris merged commit ace3533 into sveltejs:master Nov 10, 2019
@Rich-Harris
Copy link
Member

This is clever as hell. I made a small tweak, turning it into a helper:

const guard = loop_guard();

while (true) {
  foo();
  guard();
}

@Conduitry
Copy link
Member

Do we really want this applied all the time we're running code in dev mode? I can see why it might be nice in the REPL, but this seems like a pretty big change in behavior in general. Yes, people probably shouldn't be writing loops that hog the thread for 100ms but this seems like it could cause some really confusing-to-debug stuff.

@tanhauhau
Copy link
Member Author

Originally I was thinking about extra compile_options, but not sure anyone except repl would use it.

Maybe a loop_guard default true for dev, but possible to opt out?


Now you mentioned it, I realised an edge usage I might have missed,

async function () {
   while (true) {
       await foo();
   }
}

@Conduitry
Copy link
Member

Thinking about this more, I'm pretty sure I'd consider adding this a breaking change. It's a clever way of handling this, but I really think it should be opt-in.

What if there was a compiler option (probably which is only considered in dev mode) for the number of milliseconds after which loops will time out? The default would continue to be never timing out. No breaking change, and people can tune it to their needs.

@tanhauhau
Copy link
Member Author

yea i think i can make this change. what should i call this new compile option then?

🤔 loop_guard_timeout ?

@Rich-Harris
Copy link
Member

Confusingly, we use camelCase for compiler options (e.g. customElement) for consistency with JS ecosystem norms, even while we cavalierly disregard those norms internally. So I guess loopGuardTimeout, unless anyone has alternative suggestions for the name

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.

Solve the halting problem
3 participants