Open puzzledShark opened 9 years ago
Yeah, good catch. We probably need to hook into the "auto-run JavaScript" checkbox in Thimble to enable/disable running the scripts for this case. Can you dig into how it works in Thimble?
seems like this is a problem in http://jsbin.com/ too lol
Yeah, but it's solved in Thimble, if we just do what they do.
https://www.youtube.com/watch?v=EA74ODg1qKE and http://codepen.io/quezo/javascriptn/improving-infinite-loop-detection are interesting prior art.
https://github.com/jsbin/loop-protect is what jsbin does. It's an interesting situation for us, since we have acorn already in our tree, and could do something with better AST matching. A lot of work, though.
I asked the author of Acorn about whether there is code that already does this, and he gave me two other links we could think about:
The JavaScriptCodeHints plugin already has Acorn in it, so we're currently loading it. Maybe we can extend it?
is this problem already solved or nobody took the time to finish it?
very much still a thing, this work basically fell off the roadmap
I'll give it a look this week and see if its too complicated or not for me.
@Pomax the box in which users type code (where they can accidently make an infinite loop), what is that? src/editor? I'm going to look at the code of the object the user is in for writing scripting codes.
Look at src/Editor/Editor.js
, which is where our CodeMirror instances live.
thank you.
There should be a way to determine if the code is regular variables vs. [ for | do | while ] statements. Is that what the mode is?? https://github.com/mozilla/brackets/blob/master/src/editor/Editor.js#L799-L809
As far as looking at what's being typed in the editor, I'm thinking I could possibly insert some code around: https://github.com/mozilla/brackets/blob/master/src/editor/Editor.js#L865-L882
No, the mode in that code tells you "the kind of document currently open" so that CodeMirror can apply specific plugins as necessary (e.g "plain text", "xml", etc). Your second link might be useful though.
We have two places where we can detect this:
Catching it in brackets is preferable because it lets us add error notices etc much easier, so you probably want to intercept that changelist
payload and examine it for whether it contains infinite loops. You don't want to do that yourself, you want to run the javascript through something like esprima which can turn it into an AST, and then you look for potential infinite loops in that, either setting a flag that we can use on the Thimble side to not send the data over to the preview iframe, or injecting code that "makes the loops safer", but that could interfere with a user's JS.
So really step one is: can we detect it. Not even "and then how do we fix it", but just an outright "can we put any code in place that can analyse code for infinite loops and do anything if it does; like a console.warn("saw an infinite loop in usercode at line 12345 - please verify this after you stop script execution 30 seconds")
or something similarly functionally meaningless, but giving us a clear signal we now have detection. And then we can build execution mitigation on top of that detection capability as a follow-up task.
okay, I'll discuss with David tomorrow.
If the user mistakenly types in a while statement that is true in any fashion, brackets will lock up, this issue also persists in mozilla thimble