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.

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

Fixed issue #604: Kernel32#GetLastError() always returns ERROR_SUCCESS #610

Merged
merged 1 commit into from
Mar 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Bug Fixes
* [#578](https://github.com/java-native-access/jna/pull/578): Fix COM CallbackHandlers, allow usage of VARIANTs directly in c.s.j.p.w.COM.util.ProxyObject and fix native memory leak in c.s.j.p.w.COM.util.ProxyObject - [@matthiasblaesing](https://github.com/matthiasblaesing)
* [#601](https://github.com/java-native-access/jna/pull/601): Remove COMThread and COM initialization from objects and require callers to initialize COM themselves. Asserts are added to guard correct usage. - [@matthiasblaesing](https://github.com/matthiasblaesing).
* [#602] https://github.com/java-native-access/jna/pull/602): Make sure SID related memory is properly released once no longer required [@lgoldstein](https://github.com/lgoldstein).
* [#610](https://github.com/java-native-access/jna/pull/610): Fixed issue #604: Kernel32#GetLastError() always returns ERROR_SUCCESS [@lgoldstein](https://github.com/lgoldstein).

Release 4.2.1
=============
Expand Down
29 changes: 17 additions & 12 deletions contrib/platform/test/com/sun/jna/platform/win32/Kernel32Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ public static void main(String[] args) {
junit.textui.TestRunner.run(Kernel32Test.class);
}

// see https://github.com/java-native-access/jna/issues/604
public void testGetLastErrorNativeLibraryOverride() {
assertFalse("Unexpected success", Kernel32.INSTANCE.CloseHandle(null));
assertEquals("Mismatched error code", WinError.ERROR_INVALID_HANDLE, Kernel32.INSTANCE.GetLastError());
}

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

// see https://github.com/twall/jna/issues/482
public void testNoDuplicateMethodsNames() {
Collection<String> dupSet = AbstractWin32TestSupport.detectDuplicateMethods(Kernel32.class);
Expand Down Expand Up @@ -295,11 +301,10 @@ public void testGetCurrentThreadId() {

public void testGetCurrentThread() {
HANDLE h = Kernel32.INSTANCE.GetCurrentThread();
assertNotNull(h);
assertFalse(h.equals(0));
// CloseHandle does not need to be called for a thread handle
assertFalse(Kernel32.INSTANCE.CloseHandle(h));
assertEquals(WinError.ERROR_INVALID_HANDLE, Kernel32.INSTANCE.GetLastError());
assertNotNull("No current thread handle", h);
assertFalse("Null current thread handle", h.equals(0));
// Calling the CloseHandle function with this handle has no effect
assertTrue(Kernel32.INSTANCE.CloseHandle(h));
}

public void testOpenThread() {
Expand All @@ -316,11 +321,10 @@ public void testGetCurrentProcessId() {

public void testGetCurrentProcess() {
HANDLE h = Kernel32.INSTANCE.GetCurrentProcess();
assertNotNull(h);
assertFalse(h.equals(0));
// CloseHandle does not need to be called for a process handle
assertFalse(Kernel32.INSTANCE.CloseHandle(h));
assertEquals(WinError.ERROR_INVALID_HANDLE, Kernel32.INSTANCE.GetLastError());
assertNotNull("No current process handle", h);
assertFalse("Null current process handle", h.equals(0));
// Calling the CloseHandle function with a pseudo handle has no effect
assertTrue(Kernel32.INSTANCE.CloseHandle(h));
}

public void testOpenProcess() {
Expand All @@ -332,8 +336,9 @@ public void testOpenProcess() {
}

public void testQueryFullProcessImageName() {
HANDLE h = Kernel32.INSTANCE.OpenProcess(0, false, Kernel32.INSTANCE.GetCurrentProcessId());
assertNotNull("Failed (" + Kernel32.INSTANCE.GetLastError() + ") to get process handle", h);
int pid = Kernel32.INSTANCE.GetCurrentProcessId();
HANDLE h = Kernel32.INSTANCE.OpenProcess(0, false, pid);
assertNotNull("Failed (" + Kernel32.INSTANCE.GetLastError() + ") to get process ID=" + pid + " handle", h);

try {
char[] path = new char[WinDef.MAX_PATH];
Expand Down
7 changes: 6 additions & 1 deletion src/com/sun/jna/NativeLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,12 @@ private NativeLibrary(String libraryName, String libraryPath, long handle, Map<S
synchronized(functions) {
Function f = new Function(this, "GetLastError", Function.ALT_CONVENTION, encoding) {
@Override
Object invoke(Object[] args, Class<?> returnType, boolean b) {
Object invoke(Object[] args, Class<?> returnType, boolean b, int fixedArgs) {
Copy link
Member

Choose a reason for hiding this comment

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

What's this one about?

Copy link
Contributor

Choose a reason for hiding this comment

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

I recently changed the lowest-level "invoke" signature when handling interface-based mappings in order to accommodate varargs correctly, as part of a varargs fix for arm. This is transparent to any existing usage, but I neglected to update the signature on this GetLastError short-circuit, with the result being that some (all?) calls directly to GetLastError would no longer be short-circuited.

Copy link
Member

Choose a reason for hiding this comment

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

Got it.

return Integer.valueOf(Native.getLastError());
}

@Override
Object invoke(Method invokingMethod, Class<?>[] paramTypes, Class<?> returnType, Object[] inArgs, Map<String, ?> options) {
return Integer.valueOf(Native.getLastError());
}
};
Expand Down