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

infinite loop false positives #174

Closed
shiffman opened this issue Nov 2, 2016 · 11 comments
Closed

infinite loop false positives #174

shiffman opened this issue Nov 2, 2016 · 11 comments

Comments

@shiffman
Copy link
Member

shiffman commented Nov 2, 2016

I'm noticing this week in office hours (especially as students have nested loops iterating over pixels) that almost everyone has //noprotect in their code. It looks like maybe we need to extend the infinite loop detection to a little bit longer of a threshold?

@catarak
Copy link
Member

catarak commented Nov 2, 2016

The threshold is currently set to 100ms. I'll play around with some longer times--do you have any examples of code that's triggering the infinite loop that shouldn't?

catarak added a commit that referenced this issue Nov 2, 2016
@catarak
Copy link
Member

catarak commented Nov 2, 2016

This has been increased to 500ms and deployed--let me know if this seems better!

@shiffman
Copy link
Member Author

I've tested this and I'm not getting false positives anymore with nested pixel loops of typical sizes. I can still make it trigger if I am trying to iterate over a very high resolution image, but this is an edge case since it doesn't run at a reasonable speed anyway. So closing for now, we can always revisit.

@runemadsen
Copy link
Member

I am teaching a class on repetition this week, and the //noprotect warning was triggered in almost every example we did in class. On top of this, every student has //noprotect in their code when submitting homework.

Given that this is to prevent the users browser from triggering an endless loop and that it won't bog down your server, why isn't this set much, much higher? 500ms seems very aggressive given the fact that many use P5.js for pixel manipulation, etc. What's the case against doing 5000ms?

@catarak
Copy link
Member

catarak commented Nov 19, 2018

@runemadsen no reason in particular! the default for the loop-protect library is 100ms, so i thought it made sense to keep it close to that number. Since the web editor assumes usage with p5, which can include lots of examples with long loops, i think it could be raised.

i think the downside of raising it is that it would be a longer delay for triggering the loop protection for cases when it is helpful. i can imagine scenarios in which this would be annoying, so i think it will take some experimentation to figure out what a good amount of time is.

since the loop-protect library needs to be updated anyway (see #698), i'm going to reopen this ticket as part of that effort.

@catarak catarak reopened this Nov 19, 2018
@catarak catarak added this to the Post-Release milestone Nov 19, 2018
@charlie-openschool
Copy link

I notice that if I have a loop that executes for longer than the 500ms threshold I get the expected Exiting potential infinite loop exception unless I alter the code and remove a semicolon at the end of a line. The altered code will run to completion even though the elapsed time is greater than 500ms. Here is a simple example that demonstrates the behavior: https://editor.p5js.org/charlie-openschool/sketches/kGLK95CWa Is this expected? I would like to better understand this behavior.

@catarak
Copy link
Member

catarak commented Apr 2, 2019

@charlie-openschool that is weird behavior! the loop-protect library has been updated since i added it to this project (i had to fork it in order to change the threshold time), and hopefully updating it will fix this.

@catarak catarak added this to Infinite Loop Detection Refactor in All Projects Dec 11, 2019
@paraclete-pizza
Copy link

I wanted to chime in that this is still an issue. I'm updating some of my online Processing resources to P5 so kids at home with school-supplied Chromebooks can use them, and found that P5 throws an infinite loop error for pretty much any nested loop. It's not just hitting for intense cases like pixel iteration Daniel mentioned, but even the most rudimentary nested loops, like this test case:

function setup() {
createCanvas(400, 400);
}

function draw() {
background(220);
for (var x=0; x<=width; x++)
{
for (var y=0; y<=height; y++)
{
text(x, x*10, y*10);
}
}
}

@runemadsen
Copy link
Member

We had similar problems yesterday with the ccapture library that throws timeout errors in the editor but works fine locally.

@catarak
Copy link
Member

catarak commented May 7, 2020

thanks for the updates! i know this is super annoying, and i'm going to shift updating the loop protect library up in priority (see #698).

@catarak
Copy link
Member

catarak commented Feb 9, 2021

I'm going through and closing old issues/deduplicating them! Going to merge this into #698, the heart of this issue is that the library needs to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
All Projects
Infinite Loop Detection Refactor
Development

No branches or pull requests

5 participants