Update W32Service.stopService() to be more resilient to large dwWaitH…#1058
Update W32Service.stopService() to be more resilient to large dwWaitH…#1058matthiasblaesing merged 3 commits intojava-native-access:masterfrom
Conversation
…int values -- make stopService(long timeout) public so that 30 seconds is not the only valid timeout -- never wait more than the current timeout -- if the service stops during the last sleep, do not throw an exception -- use the preexisting timing logic in waitForNonPendingState to sleep fractions of the total dwWaitHint instead of the entire time
|
Thank you Keith, the change looks sane to me, before merging this, I have one question: Was this tested? If so how? |
|
Pull request has been updated with the CHANGES.md update. Testing was primarily conducted against a prorun driven service I had source to, where I could control I also verified the routine still worked against a few other random services. |
| throw new RuntimeException(String.format("Service stop exceeded timeout time of %d ms", timeout)); | ||
| } | ||
|
|
||
| int dwWaitTime = Math.min(sanitizeWaitTime(status.dwWaitHint), (int)msRemainingBeforeTimeout); |
There was a problem hiding this comment.
This needs to be declared as a long. The user could pass in a timeout value > Integer.MAX_VALUE, so msRemainingBeforeTimeout could also be > Integer.MAX_VALUE. After the cast, the value will wrap around and you will get negative wait time.
There was a problem hiding this comment.
Agreed. Done and committed.
|
Thank you - looks good to me. |
…int values
-- make stopService(long timeout) public so that 30 seconds is not the only valid timeout
-- never wait more than the current timeout
-- if the service stops during the last sleep, do not throw an exception
-- use the preexisting timing logic in waitForNonPendingState to sleep fractions of the total dwWaitHint instead of the entire time
-- get rid of some leftover code that no longer served a purpose