False positive for indefinite loop with multiple conditions (javascript:S2189)

I have a loop which has two exit parameters:

  • One is a flag that is set from the outside
  • One is a “timeout” check

The loop looks like:

var timeOutInSeconds = 300;
var startTime = Date.now();
while (shouldLoopRun && Date.now() - startTime < timeOutInSeconds * 1000) {
    ...
}

Sonar now complains with javascript:S2189 that shouldLoopRun is not modified in the loop. That is correct (and wanted) as it is modified outside the loop (asynchronously). And this should anyway not be an issue as there is another way to exit this loop (with the timeout).

Hello Roman,

Welcome to the SonarSource community, and thank you for your message!

I understand the point you are raising, and I think it’s worth creating a ticket to consider this case. However, before doing so, I would appreciate if you could further clarified with an updated version of your code snippet how the flag is asynchronously modified outside of the loop.

This will help me better define the scope of the ticket and highlight what kind of fix is actually needed.

Thanks,
Yassin

Thanks for coming back.

Here is a complete sample:

shouldLoopRun = true;

async function main() {
  var timeOutInSeconds = 10;
  var startTime = Date.now();
  while (shouldLoopRun && Date.now() - startTime < timeOutInSeconds * 1000)
  {
    console.log("bla");
    await sleep(1000);
  }
}

async function loop() {
  await sleep(5000);
  shouldLoopRun = false;
}

function sleep(ms) {
  return new Promise(resolve => setTimeout(resolve, ms));
}

main();
loop();

As you see, the loop gets cancelled after 5 seconds because the loop method sets the flag that cancels the loop.
I see that this is a very very hard method to find for sonarqube and when just using this flag, the Sonar message is actually correct.
But as you see, there is a second condition that is based on Date.now() which cancells the loop after 10 seconds in any case without additional logic outside the loop. And I think that should be detected by Sonar to prevent the warning that is shown.

Hello Roman,

Thanks for the updated snippet. I now understand that there are actually two kinds of false-positive.

In that regard, I created these two tickets: #2743 and #2744.

Regards,
Yassin

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.