blockbuilder icon indicating copy to clipboard operation
blockbuilder copied to clipboard

Writing loops can crash the browser/tab

Open ericsoco opened this issue 8 years ago • 7 comments

I don't have a clear repro for this (and am afraid to try!) but: I was writing a for loop and the tab became unresponsive (Chrome). I think I probably piled up a bunch of script refresh requests (or maybe just one is all it took?) that had infinite loops in them, something like the unfinished:

for (var i=0; i<10; i) {}

(The braces are autocompleted in the editor, which makes it possible for the above incomplete construction to get compiled.)

Without having looking into Block Builder's source, I'm not sure how you might detect and kill this kind of process, but you might put a timeout on each script rebuild and somehow break out of the build if it takes too long...

ericsoco avatar Dec 18 '15 00:12 ericsoco

You're correct that the incomplete infinite loop may get executed, with no break out.

As we all know, the halting problem is unsolvable, but we can try anyway.

We explored this issue some in Tributary (https://github.com/enjalot/tributary) -- we considered using esprima to modify the AST with depth counters. I worked on a fork on this some, but did not complete it.

JSBin inserts a timer in loops to break if it's too long (https://github.com/jsbin/loop-protect) using a regex simply inserting code manually into the source. Last time I used this, the version on github did not work as well as the one actually used on JSBin (there were a number of cases that I found still resulted in infinite loops)

A note: if we insert code manually, the code executed will not be the same as what is written. We need to be careful to preserve line numbers so that errors reported from the execution of the code still reflect the original source code.

georules avatar Dec 18 '15 01:12 georules

Do any IDE's use web workers (service workers) to isolate the potentially errant client script's runtime? Could such an approach allow an IDE to function normally even when the script locks up it's worker?

I'm not sure if one incurs a performance hit when using web workers for such a purpose, but for testing and development, running the JavaScript code in a separate worker might avoid the problem @ericsoco brings up.

EDIT:

In other words, rather than trying to be clever, just give the script a sandbox to play in; watch it maybe kill it if something goes wrong, such as an infinite loop.

bmershon avatar Dec 18 '15 03:12 bmershon

Alternatively, web workers might be used as a watchdog script, pinging the main script and halting it if there is no response after N ms. (Again, I haven't dug into the source, so I'm not sure this is feasible...just an idea.)

ericsoco avatar Dec 18 '15 17:12 ericsoco

I like the webworkers idea. I'll see if I can prototype it.

georules avatar Dec 19 '15 21:12 georules

Another solution is to save all scripts to localStorage before running the iframe:

diff --git a/public/js/components/renderer.js b/public/js/components/renderer.js
index 23ba43d..49618d0 100644
--- a/public/js/components/renderer.js
+++ b/public/js/components/renderer.js
@@ -40,6 +40,21 @@ var Renderer = React.createClass({

       var doRefresh1 = this.props.active === "README.md" || prevProps.active === "README.md";
       var doRefresh2 = this.props.active === prevProps.active;
+
+      // save to localStorage
+      if (typeof localStorage == 'object') {
+        var f;
+        for (f in this.props.gist.files) {
+          if (f.match(/\.(html|js|md)$/)
+          && this.props.gist.files[f].content
+          && this.props.gist.files[f].content.length > 0) {
+            localStorage.setItem(
+              `${this.props.description}: ${f}`,
+               this.props.gist.files[f].content
+            );
+          }
+        }
+      }
       if (doRefresh1 || doRefresh2) {
         this.setupIFrame();

This way if the browser crashes, we have a way to copy/paste our code from the Application tab in the dev tools.

Tested locally with the infinite loop:

capture d ecran 2017-02-02 23 27 13

I don't think saving html and js files each time will be too slow, but if it is too slow, we can maybe do it once in a while (even once every 20 keystrokes would be great).

Fil avatar Feb 02 '17 22:02 Fil

EDIT: sorry I'm such a joke, this patch doesn't work when you crash and the dev tools are not open. Why? because to get to your locally saved code, you need to open the dev tools, which means that you will need (or want) to reload the page… which then saves older code to localStorage… and erases your shiny (and infinitely buggy) code. So there is a little bit of something more to do in order to prevent overwriting saved content.

Fil avatar Feb 02 '17 22:02 Fil

Here's a version that keeps up to 5 saves of each file. I have no idea for a better UX; "go quickly into Dev Tools > Application > Storage to get your code" is not ideal, but still better than losing all.

      // save to localStorage
      if (typeof localStorage == 'object') {
        var f;
        for (f in this.props.gist.files) {
          if (f.match(/\.(html|js|md)$/)
          && this.props.gist.files[f].content
          && this.props.gist.files[f].content.length > 0) {
            var name = `${this.props.description}: ${f}`,
                content = this.props.gist.files[f].content,
                i = localStorage.length;
            // keep up to 5 older (and different) saves
            Object.keys(localStorage)
            .filter(function(d) { return d.substring(0,name.length) === name; })
            .filter(function(d) { if (localStorage.getItem(d) == content) {
                localStorage.removeItem(d);
                return false;
            } else {
                return true;
            } })
            .sort(function(a, b) { return d3.descending(a,b); })
            .slice(5)
            .map(function(d) { localStorage.removeItem(d); });
            localStorage.setItem(`${name}.${+ new Date()}`, content);
          }
        }

Fil avatar Feb 05 '17 12:02 Fil