Added changing optional config of W32 services #489

akandi · 2015-08-20T13:19:45Z

This allows to add failure configurations to Windows services.

dblock · 2015-08-20T14:29:44Z

These most definitely need working tests. Note #258 too.

twall · 2015-08-20T15:01:37Z

contrib/platform/src/com/sun/jna/platform/win32/Advapi32.java

+	 *         If the function fails, the return value is zero. To get extended
+	 *         error information, call GetLastError.
+	 */
+	public boolean ChangeServiceConfig2W(SC_HANDLE hService, int dwInfoLevel,


Is the -W suffix necessary? Why?

akandi · 2015-08-20T15:14:51Z

The tests work, but I commented them out analogous to most other tests in W32ServiceTest.java because they edit real services.

How do I proceed concerning #258? Should I merge my changes into #258 and work from there?

The W-suffix seems necessary to me because the ChangeServiceConfig2 uses TCHAR strings (LPTSTR) for at least some configuration options (namely in the SERVICE_FAILURE_ACTIONS struct).
I didn't see a way of finding out from Java whether the binary that is used was compiled with UNICODE defined or not.

dblock · 2015-08-20T15:46:15Z

JNA will automatically deal with the Ws, remove that. UNICODE is default.

Lets begin with the smallest PR possible, this is good to start, but you do need tests for these bindings. Windows has some default services that are always there, and some functions can be called all the time, and you might find some inspiration in service tests of https://github.com/dblock/msiext. All we want in JNA tests is that each Win32 API added has a test that invokes it. Often invoking it and getting back an expected error is good enough.

To be clear, first add a test for ChangeServiceConfig2W.

akandi · 2015-08-21T09:11:37Z

At akandi@53f0f6c I started writing a better test. Is this along the lines of what you imagined?

However I seem to be missing something, maybe the alignment upon conversion back to Structure is off, and the test fails. I have placed a dump of the buffer after a call to QueryServiceConfig2 in contrib/platform/dump.txt and it looks quite right.

akandi · 2015-08-21T13:51:02Z

The issue is resolved. I didn't realize I had to call read on the freshly created structure from pointer. Both tests run fine now (provided that the user has sufficient permissions to edit services).

twall · 2015-08-22T00:15:20Z

contrib/platform/src/com/sun/jna/platform/win32/W32Service.java

+	public SERVICE_FAILURE_ACTIONS getFailureActions() {
+		Pointer buffer = queryServiceConfig2(Winsvc.SERVICE_CONFIG_FAILURE_ACTIONS);
+		SERVICE_FAILURE_ACTIONS result = new SERVICE_FAILURE_ACTIONS(buffer);
+		result.read();


You should make the read() call part of the SERVICE_FAILURE_ACTIONS(Pointer) constructor.

akandi · 2015-08-24T07:09:48Z

I'm not sure if this last commit is necessary. If I comment out the part of the test that reverts the service changes then if I simply use Strings in the struct I end up with Chinese characters in my service configuration. If I use WString all is fine, obviously, but I don't know if that version is compatible with non-Unicode systems.

lgoldstein · 2015-08-24T12:29:14Z

contrib/platform/src/com/sun/jna/platform/win32/Advapi32.java

+	 *         error information, call GetLastError.
+	 */
+	public boolean ChangeServiceConfig2(SC_HANDLE hService, int dwInfoLevel,
+			Structure lpInfo);


Using the generic Structure is a bit too general - whenever the API expects several variants we "overload" the method - see GetVersionEx as an example. In this case there should be a match between the dwInfoLevel and the actual structure type, but it is up to the user to make sure that the level matches the type. Another alternative would be to define an abstract ServiceConfigStructure that extends Structure and have all the possible variants extend the ServiceConfigStructure - this would allow some degree of type safety...

Yes, please make one variant per supported structure type. Since I don't believe there is any shared structure between the service config structs, using a ServiceConfigStructure would probably be not much better than Structure.

akandi · 2015-08-24T13:55:52Z

@twall like so?
When using Winsvc.QueryServiceConfig2 the contents of this struct will be filled by Windows, so it is used to transport Strings in both directions (to&from JNA).
Using String in SERVICE_FAILURE_ACTIONS still results in Chinese characters in my service configuration with this last commit.

@lgoldstein @twall Makes sense to me. But I see that in #258 they have gone the route of having an abstract superclass (namely ChangeServiceConfig2Info).

twall · 2015-08-25T12:36:56Z

@akandi yes, that's what I mean. The TypeMapper involved there works both directions, but in order for the type mapper to be used, the Structure must be defined within the Library interface (or have the TypeMapper attached directly to the struct, which is less preferable).

Is there a test I can run to expose the Chinese character problem?

As for the structure base class, it can be useful to communicate that the structure variants are used in the same place, and if there are a lot of them it's easier to define a single method mapping. I don't have a strong opinion there, so I'd say pick what seems to make things clearest to the end user.

akandi · 2015-08-25T13:37:33Z

So the SERVICE_FAILURE_ACTIONS Structure - as well as others that depend on the TypeMapper - would have to be moved from the Winsvc to the Advapi32 interface?

The Chinese characters appear in my Service configuration (services.msc, right click on Windows Time, Properties, Recoveries tab, in the Run program section the value of the Program text box is Chinese characters) after having the config set through JNA .
I just conducted a test using a C++ program and both setting a wide string through ChangeServiceConfig2W and setting a non-wide string through ChangeServiceConfig2A produced correct result in the Service configuration.
Thus I conclude that a mismatch of the String width in the Structure and the native function that is called occurs. Could be just a problem on my side, I don't know.

twall · 2015-08-26T00:39:42Z

Only the SERVICE_FAILURE_ACTIONS structure would need to be moved, since it's the only one that has String fields. Alternatively you can explicitly set a type mapper in its constructor (select W32APITypeMapper.UNICODE|ASCII as is done in W32APIOptions).

The method for finding a structure's type mapper (if one is not explicitly set) is to check the enclosing class for an instance of Library which has a field of the same type as the enclosing class. In the case of Winsvc as the enclosing class, no type mapper would be found automatically.

I notice there are a few Structures in Winbase.java, WinCrypt.java, and WinUser.java which would have this same problem (defined within a class which has no instantiable library instance from which to pull a TypeMapper).

twall · 2015-12-04T10:47:17Z

This behavior should be more consistent now with recent changes to both the w32 API library hierarchy and fixes to tracking appropriate library options.

twall · 2015-12-04T11:55:48Z

Conflicts resolved and pushed to branch akandi-failureConfig. Will test this on w32 when I get a chance.

twall · 2016-01-19T04:45:37Z

@akandi Please re-test any local setup you might have based on the new master.

Motivation: Let's use the latest maven version to build Modifications: Upgrade to 3.9.0 Result: Use latest maven release

akandi added 2 commits August 20, 2015 13:29

implemented failure configuration of windows services

909d68b

add privileges if necessary

0165741

twall reviewed Aug 20, 2015
View reviewed changes

akandi added 2 commits August 21, 2015 15:30

better tests for w32 optional service config

2b8b0a8

removed obsolete hex dump

40abaef

twall reviewed Aug 22, 2015
View reviewed changes

akandi added 2 commits August 24, 2015 08:37

moved struct reads to constructors

472ab5e

manual handling of wchar vs char

c3e540b

lgoldstein reviewed Aug 24, 2015
View reviewed changes

changed library loads in platform.win32 to W32APIOptions.DEFAULT_OPTIONS

35277d1

akandi added 2 commits August 26, 2015 09:51

use correct type mapper for SERVICE_FAILURE_ACTIONS

fe2e9dd

revert WString parameters in Advapi32.java

c092b77

twall added a commit that referenced this pull request Dec 4, 2015

fix merge conflicts for #489

b705ecb

twall merged commit c092b77 into java-native-access:master Jan 19, 2016

mstyura pushed a commit to mstyura/jna that referenced this pull request Sep 9, 2024

Update maven to 3.9.0 (java-native-access#489)

3c607fb

Motivation: Let's use the latest maven version to build Modifications: Upgrade to 3.9.0 Result: Use latest maven release

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added changing optional config of W32 services #489

Added changing optional config of W32 services #489

akandi commented Aug 20, 2015

dblock commented Aug 20, 2015

twall Aug 20, 2015

akandi commented Aug 20, 2015

dblock commented Aug 20, 2015

akandi commented Aug 21, 2015

akandi commented Aug 21, 2015

twall Aug 22, 2015

akandi commented Aug 24, 2015

lgoldstein Aug 24, 2015

twall Aug 24, 2015

akandi commented Aug 24, 2015

twall commented Aug 25, 2015

akandi commented Aug 25, 2015

twall commented Aug 26, 2015

twall commented Dec 4, 2015

twall commented Dec 4, 2015

twall commented Jan 19, 2016

Added changing optional config of W32 services #489

Added changing optional config of W32 services #489

Conversation

akandi commented Aug 20, 2015

dblock commented Aug 20, 2015

twall Aug 20, 2015

Choose a reason for hiding this comment

akandi commented Aug 20, 2015

dblock commented Aug 20, 2015

akandi commented Aug 21, 2015

akandi commented Aug 21, 2015

twall Aug 22, 2015

Choose a reason for hiding this comment

akandi commented Aug 24, 2015

lgoldstein Aug 24, 2015

Choose a reason for hiding this comment

twall Aug 24, 2015

Choose a reason for hiding this comment

akandi commented Aug 24, 2015

twall commented Aug 25, 2015

akandi commented Aug 25, 2015

twall commented Aug 26, 2015

twall commented Dec 4, 2015

twall commented Dec 4, 2015

twall commented Jan 19, 2016