javadelight / delight-nashorn-sandbox

A sandbox for executing JavaScript with Nashorn in Java.
Other
268 stars 81 forks source link

ThreadMonitor can enter extended wait on monitor without first checking that the evaluation isn't already complete #153

Closed LG987234122 closed 2 months ago

LG987234122 commented 2 months ago

Within ThreadMonitor.run, there's code of the form:

synchronized (monitor) {
      long waitTime = getCheckInterval(runtime);

      if (waitTime == 0) {
          waitTime = 1;
      }
      monitor.wait(waitTime);
  }

The problem here is that if the JS evaluation has completed just prior to entering the synchronized block, the monitor thread won't notice and enter its extended wait. Placing a check for stop within the synchronized block allows the monitor thread to notice the JS evaluation is already complete, avoiding the unecessary wait.

synchronized (monitor) {
          if (stop.get())
          {
        return;
          }

      long waitTime = getCheckInterval(runtime);

      if (waitTime == 0) {
          waitTime = 1;
      }
      monitor.wait(waitTime);
  }
mxro commented 2 months ago

Thank you for raising this issue and suggesting the fix!

The changes has been applied and releases with 0.4.4