Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update W32Service.stopService() to be more resilient to large dwWaitH… #1058

Merged
merged 3 commits into from
Jan 22, 2019
Merged

Update W32Service.stopService() to be more resilient to large dwWaitH… #1058

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f50c06e
Update W32Service.stopService() to be more resilient to large dwWaitH…
keithharp-nice Jan 18, 2019
4a0c40d
Add info to CHANGES.md
keithharp-nice Jan 21, 2019
9303523
Change int to long to avoid possible overflow condition
keithharp-nice Jan 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Next release (5.3.0)

Features
--------
* [#1058](https://github.com/java-native-access/jna/pull/1058): Add selectable timeout to stopService() and improve timeout handling - [@keithharp](https://github.com/keithharp).
* [#1050](https://github.com/java-native-access/jna/pull/1050): Add `c.s.j.p.win32.VersionHelpers` and supporting functions - [@dbwiddis](https://github.com/dbwiddis).

Bug Fixes
Expand Down
49 changes: 28 additions & 21 deletions contrib/platform/src/com/sun/jna/platform/win32/W32Service.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public void stopService() {
* @param timeout timeout in ms until the service must report to be
* stopped
*/
void stopService(long timeout) {
public void stopService(long timeout) {
long startTime = System.currentTimeMillis();
waitForNonPendingState();
// If the service is already stopped - return
Expand All @@ -236,22 +236,23 @@ void stopService(long timeout) {
// the previouos implementation queried the service status and
// failed if the application did not correctly update its state
while(status.dwCurrentState != Winsvc.SERVICE_STOPPED) {
try {
Thread.sleep( status.dwWaitHint );
long msRemainingBeforeTimeout = timeout - (System.currentTimeMillis() - startTime);

if (msRemainingBeforeTimeout < 0) {
throw new RuntimeException(String.format("Service stop exceeded timeout time of %d ms", timeout));
}

int dwWaitTime = Math.min(sanitizeWaitTime(status.dwWaitHint), (int)msRemainingBeforeTimeout);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Done and committed.


try {
Thread.sleep( dwWaitTime );
} catch (InterruptedException e){
throw new RuntimeException(e);
}
if(! Advapi32.INSTANCE.QueryServiceStatus(_handle, status)) {
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
}
if((System.currentTimeMillis() - startTime) > timeout) {
throw new RuntimeException(String.format("Service stop exceeded timeout time of %d ms", timeout));
}
}
waitForNonPendingState();
if (queryStatus().dwCurrentState != Winsvc.SERVICE_STOPPED) {
throw new RuntimeException("Unable to stop the service");
}
}

/**
Expand Down Expand Up @@ -292,7 +293,22 @@ public void pauseService() {
}
}

/**
/**
* do not wait longer than the wait hint. A good interval is
* one-tenth the wait hint, but no less than 1 second and no
* more than 10 seconds.
*/
int sanitizeWaitTime(int dwWaitHint) {
int dwWaitTime = dwWaitHint / 10;

if (dwWaitTime < 1000)
dwWaitTime = 1000;
else if (dwWaitTime > 10000)
dwWaitTime = 10000;
return dwWaitTime;
}

/**
* Wait for the state to change to something other than a pending state.
*/
public void waitForNonPendingState() {
Expand All @@ -315,16 +331,7 @@ public void waitForNonPendingState() {
throw new RuntimeException("Timeout waiting for service to change to a non-pending state.");
}

// do not wait longer than the wait hint. A good interval is
// one-tenth the wait hint, but no less than 1 second and no
// more than 10 seconds.

int dwWaitTime = status.dwWaitHint / 10;

if (dwWaitTime < 1000)
dwWaitTime = 1000;
else if (dwWaitTime > 10000)
dwWaitTime = 10000;
int dwWaitTime = sanitizeWaitTime(status.dwWaitHint);

try {
Thread.sleep( dwWaitTime );
Expand Down