Proposal: loop guards.

[erikh2000]

I've now found 3 separate causes for infinite loops--2 of these I've already reported. The third, I'm narrowing down the repro still.

Of course, we can work through fixing the bugs that cause the infinite loops. And we should. But I propose that we add some simple guards to the loops that check for an excessive loop count and exit out before the JS stack is depleted. My reasoning is as follows:

• Troubleshooting bugs is easier because the function can fail BEFORE the JS execution environment crashes, and allow us to log the error and context, e.g. the params that were passed to the failing call.
• The APIs can be called within an app even if they aren't completely free of bugs. I am okay with a function failing 1 out of every 100 times I call it, if the app can recover. But currently, when run inside of a web app, a user would see our entire web page replaced with an error message as a result of the JS execution environment crashing. In a production app, this is the kind of thing that gets people woke up at 3am to come in and fix.

(This image below is currently what the end user of our web app would see after one failed call to a martinez API)

It would be a pretty reasonable response to say "we're in beta now. We're going to fix these bugs, and then there would be no need for the inelegant loop guard thing you're talking about." The thing is... W8r/Martinez is doing better RIGHT NOW than the TurfJS (JSTS-inherited) boolean functions my project is using in production. When I say "better", I mean that the cases I had that were generating weird TopologyExceptions down in the bowels of JSTS are working fine with W8R/Martinez. There is just one dealbreaking caveat... it can frigging crash the browser!

So I will probably add the loop guards in my local fork of W8R/Martinez in an attempt to get it production-usable, unless somebody gives a great reason not to (e.g. "I already did that!"). And y'all can let me know if you're interested in a PR. I'm happy to throw it up there and have the specifics of it be disagreed with--just wanted to check for interest in receiving such a PR before sending it at you.

[erikh2000]

To give a concrete example of how a loop guard could work...

module.exports = function subdivide(eventQueue, subject, clipping, sbbox, cbbox, operation) {

  ...stuff...

  var eventQueueGuard = createLoopGuard(10000, 'subdivide() event queue'); // 10000 max interations. # can be debated and adjusted.
  while (eventQueue.length) {
    eventQueueGuard.check(); // Will throw an exception if called more than 10k times in this scope.

    ...more stuff...

[rowanwins]

Gday @erikh2000

Yeah I think this proposal is heading in the right direction, I presume it would have minimal overhead on the performance. I guess the question is how we set the number - perhaps we could simply double or triple the length of the event queue as a crude first cut? Best way to test things out is to create a fork and tinker and see what works :)

[erikh2000]

Thanks for the feedback. I have it working and will post a PR tomorrow. Yes, the loop check is very minimal (basically just ´´´if (++iterationCount > maxAllowed) throw```). It will have no noticable affect on performance.

[HansBrende]

@w8r I am also running into infinite loops. See my comment here.

@erikh2000 have you found any solution or a different library to use? Please let me know.

[erikh2000]

@HansBrende could you connect with me on LinkedIn - https://www.linkedin.com/in/erikhermansen/ ? I can answer your question better there.