Bug in ThreadMonitor that can cause it to wait unnecessarily

[cmorriss]

I had noticed in some stress testing that sometimes the call to execute a function would take the maximum CPU limit time. I tracked it down to an issue in ThreadMonitor in the call to wait on the monitor object. It's possible that the threadToMonitor was already set and the monitor notification sent. In that case, the monitor wait goes to the maximum value for the cpu time limit.

The fix is pretty simple. You simply need to check if the threadToMonitor is not null in the synchronized block, but before the wait, and if it's not null, skip the wait. I can submit a pull request with the fix if that will help.

[cmorriss]

// wait, for threadToMonitor to be set in JS evaluator thread
synchronized (monitor) {
    if (threadToMonitor == null) {
        monitor.wait(maxCPUTime / MILI_TO_NANO);
    }
}

[mxro]

Hi Chris,

thanks for finding this! A pull request would be fantastic!

[cmorriss]

Updates to the pull request were to get in line with the existing formatting conventions.

[cmorriss]

Any issues with merging this request? Happy to answer any comments or update the PR if needed.

[mxro]

Fix included in release 0.1.14. Please close the issue if the fix is okay. Thank you for reporting and fixing it!!

[cmorriss]

Thanks for including and sorry for the delay in closing.