Skip to content

Commit

Permalink
Merge pull request #612 from lgoldstein/free-memory
Browse files Browse the repository at this point in the history 
Kernel32Util#freeLocal/GlobalMemory always throws Win32Exception if failed
  • Loading branch information
@dblock
dblock committed Mar 6, 2016
2 parents 5521627 + 9b7ab42 commit 0acce1f
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 97 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Features
* [#595](https://github.com/java-native-access/jna/pull/595): Allow calling COM methods/getters requiring hybrid calling (METHOD+PROPERTYGET) - [@matthiasblaesing](https://github.com/matthiasblaesing).
* [#582](https://github.com/java-native-access/jna/pull/582): Mavenize the build process - Phase 1: building the native code via Maven [@lgoldstein](https://github.com/lgoldstein).
* [#606](https://github.com/java-native-access/jna/pull/606): Added Kerne32Util method to facilitate checking that calls to LocalFree/GlobalFree are successful [@lgoldstein](https://github.com/lgoldstein).
* [#612](https://github.com/java-native-access/jna/pull/612): Kernel32Util#freeLocal/GlobalMemory always throws Win32Exception if failed [@lgoldstein](https://github.com/lgoldstein).

Bug Fixes
---------
Expand Down
8 changes: 4 additions & 4 deletions contrib/platform/src/com/sun/jna/platform/win32/Advapi32Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ public static String convertSidToStringSid(PSID sid) {
try {
return ptr.getWideString(0);
} finally {
Kernel32Util.validateFreeLocalMemory(ptr);
Kernel32Util.freeLocalMemory(ptr);
}
}

Expand All @@ -323,7 +323,7 @@ public static byte[] convertStringSidToSid(String sidString) {
try {
return value.getBytes();
} finally {
Kernel32Util.validateFreeLocalMemory(value.getPointer());
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand All @@ -347,7 +347,7 @@ public static boolean isWellKnownSid(String sidString, int wellKnownSidType) {
try {
return Advapi32.INSTANCE.IsWellKnownSid(value, wellKnownSidType);
} finally {
Kernel32Util.validateFreeLocalMemory(value.getPointer());
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand Down Expand Up @@ -2214,7 +2214,7 @@ public static Memory getSecurityDescriptorForObject(final String absoluteObjectP
memory.write(0, data, 0, nLength);
return memory;
} finally {
Kernel32Util.validateFreeLocalMemory(secValue);
Kernel32Util.freeLocalMemory(secValue);
}
}

Expand Down
22 changes: 11 additions & 11 deletions contrib/platform/src/com/sun/jna/platform/win32/Crypt32Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public static byte[] cryptProtectData(byte[] data, byte[] entropy, int flags,
return pDataProtected.getData();
} finally {
if (pDataProtected.pbData != null) {
Kernel32Util.validateFreeLocalMemory(pDataProtected.pbData);
Kernel32Util.freeLocalMemory(pDataProtected.pbData);
}
}
}
Expand Down Expand Up @@ -133,25 +133,25 @@ public static byte[] cryptUnprotectData(byte[] data, byte[] entropy, int flags,
}
} finally {
if (pDataUnprotected.pbData != null) {
int rc = Kernel32Util.freeLocalMemory(pDataUnprotected.pbData);
if (rc != WinError.ERROR_SUCCESS) {
Win32Exception exc = new Win32Exception(rc);
try {
Kernel32Util.freeLocalMemory(pDataUnprotected.pbData);
} catch(Win32Exception e) {
if (err == null) {
err = exc;
err = e;
} else {
err.addSuppressed(exc);
err.addSuppressed(e);
}
}
}

if (pDescription.getValue() != null) {
int rc = Kernel32Util.freeLocalMemory(pDescription.getValue());
if (rc != WinError.ERROR_SUCCESS) {
Win32Exception exc = new Win32Exception(rc);
try {
Kernel32Util.freeLocalMemory(pDescription.getValue());
} catch(Win32Exception e) {
if (err == null) {
err = exc;
err = e;
} else {
err.addSuppressed(exc);
err.addSuppressed(e);
}
}
}
Expand Down
49 changes: 7 additions & 42 deletions contrib/platform/src/com/sun/jna/platform/win32/Kernel32Util.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,64 +54,29 @@ public static String getComputerName() {
return Native.toString(buffer);
}

/**
* Makes sure that local memory has been successfully freed
*
* @param ptr The {@link Pointer} to the memory to be released - ignored if NULL
* @see #freeLocalMemory(Pointer)
* @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported
*/
public static void validateFreeLocalMemory(Pointer ptr) {
int rc = freeLocalMemory(ptr);
if (rc != WinError.ERROR_SUCCESS) {
throw new Win32Exception(rc);
}
}

/**
* Invokes {@link Kernel32#LocalFree(Pointer)} and checks if it succeeded.
*
* @param ptr The {@link Pointer} to the memory to be released - ignored if NULL
* @return {@code ERROR_SUCCESS} or reason for failing to free the memory
* @see Native#getLastError()
* @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported
*/
public static int freeLocalMemory(Pointer ptr) {
public static void freeLocalMemory(Pointer ptr) {
Pointer res = Kernel32.INSTANCE.LocalFree(ptr);
if (res != null) {
return Native.getLastError();
} else {
return WinError.ERROR_SUCCESS;
}
}


/**
* Makes sure that global memory has been successfully freed
*
* @param ptr The {@link Pointer} to the memory to be released - ignored if NULL
* @see #freeGlobalMemory(Pointer)
* @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported
*/
public static void validateFreeGlobalMemory(Pointer ptr) {
int rc = freeGlobalMemory(ptr);
if (rc != WinError.ERROR_SUCCESS) {
throw new Win32Exception(rc);
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
}
}

/**
* Invokes {@link Kernel32#GlobalFree(Pointer)} and checks if it succeeded.
*
* @param ptr The {@link Pointer} to the memory to be released - ignored if NULL
* @return {@code ERROR_SUCCESS} or reason for failing to free the memory
* @see Native#getLastError()
* @throws Win32Exception if non-{@code ERROR_SUCCESS} code reported
*/
public static int freeGlobalMemory(Pointer ptr) {
public static void freeGlobalMemory(Pointer ptr) {
Pointer res = Kernel32.INSTANCE.GlobalFree(ptr);
if (res != null) {
return Native.getLastError();
} else {
return WinError.ERROR_SUCCESS;
throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
}
}

Expand Down Expand Up @@ -141,7 +106,7 @@ public static String formatMessage(int code) {
String s = ptr.getWideString(0);
return s.trim();
} finally {
validateFreeLocalMemory(ptr);
freeLocalMemory(ptr);
}
}

Expand Down
39 changes: 13 additions & 26 deletions contrib/platform/test/com/sun/jna/platform/win32/Advapi32Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ public void testIsValidSid() {
assertTrue("Non positive sid length", sidLength > 0);
assertTrue("Invalid sid", Advapi32.INSTANCE.IsValidSid(value));
} finally {
assertEquals("Failed to release SID",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer()));
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand All @@ -159,8 +158,7 @@ public void testGetSidLength() {
try {
assertEquals("Wrong SID length", 12, Advapi32.INSTANCE.GetLengthSid(value));
} finally {
assertEquals("Failed to free SID",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer()));
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand Down Expand Up @@ -194,8 +192,7 @@ public void testLookupAccountSid() {
assertEquals("Everyone", nameString);
assertTrue(referencedDomainNameString.length() == 0);
} finally {
assertEquals("Failed to release sid",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer()));
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand All @@ -214,12 +211,10 @@ public void testConvertSid() {
String convertedSidString = conv.getWideString(0);
assertEquals("Mismatched SID string", convertedSidString, sidString);
} finally {
assertEquals("Failed to release string value",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(conv));
Kernel32Util.freeLocalMemory(conv);
}
} finally {
assertEquals("Failed to release sid",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer()));
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand Down Expand Up @@ -631,8 +626,7 @@ public void testIsWellKnownSid() {
assertTrue("Not a world sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinWorldSid));
assertFalse("Unexpected admin sid", Advapi32.INSTANCE.IsWellKnownSid(value, WELL_KNOWN_SID_TYPE.WinAccountAdministratorSid));
} finally {
assertEquals("Failed to release sid",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(value.getPointer()));
Kernel32Util.freeLocalMemory(value.getPointer());
}
}

Expand All @@ -652,8 +646,7 @@ public void testCreateWellKnownSid() {
String convertedSidString = conv.getWideString(0);
assertEquals("Mismatched SID string", EVERYONE, convertedSidString);
} finally {
assertEquals("Failed to release string",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(conv));
Kernel32Util.freeLocalMemory(conv);
}
}

Expand Down Expand Up @@ -1011,8 +1004,7 @@ public void testGetNamedSecurityInfoForFileNoSACL() throws Exception {
file.delete();
}
} finally {
assertEquals("Failed to free security descriptor of " + file,
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()));
Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue());
}
}

Expand Down Expand Up @@ -1084,8 +1076,7 @@ public void testGetNamedSecurityInfoForFileWithSACL() throws Exception {
file.delete();
}
} finally {
assertEquals("Failed to free security descriptor of " + filePath,
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()));
Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue());
}
if (impersontating) {
Advapi32.INSTANCE.SetThreadToken(null, null);
Expand Down Expand Up @@ -1137,8 +1128,7 @@ public void testSetNamedSecurityInfoForFileNoSACL() throws Exception {
file.delete();
}
} finally {
assertEquals("Failed to release security descriptor of " + file,
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()));
Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue());
}
}

Expand Down Expand Up @@ -1225,8 +1215,7 @@ public void testSetNamedSecurityInfoForFileWithSACL() throws Exception {
file.delete();
}
} finally {
assertEquals("Failed to release security descriptor of " + file,
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()));
Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue());
}

if (impersontating) {
Expand Down Expand Up @@ -1269,8 +1258,7 @@ public void testGetSecurityDescriptorLength() throws Exception {
file.delete();
}
} finally {
assertEquals("Failed to release security descriptor of " + file,
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()));
Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue());
}
}

Expand Down Expand Up @@ -1301,8 +1289,7 @@ public void testIsValidSecurityDescriptor() throws Exception {
file.delete();
}
} finally {
assertEquals("Failed to release security descriptor of " + file,
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue()));
Kernel32Util.freeLocalMemory(ppSecurityDescriptor.getValue());
}
}

Expand Down
18 changes: 6 additions & 12 deletions contrib/platform/test/com/sun/jna/platform/win32/Crypt32Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,13 @@ public void testCryptProtectUnprotectData() {
assertEquals("description", pDescription.getValue().getWideString(0));
assertEquals("hello world", pDataDecrypted.pbData.getString(0));
} finally {
assertEquals("Failed to free decrypted data",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataDecrypted.pbData));
Kernel32Util.freeLocalMemory(pDataDecrypted.pbData);
}
} finally {
assertEquals("Failed to free description",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDescription.getValue()));
Kernel32Util.freeLocalMemory(pDescription.getValue());
}
} finally {
assertEquals("Failed to free encrypted data",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataEncrypted.pbData));
Kernel32Util.freeLocalMemory(pDataEncrypted.pbData);
}
}

Expand All @@ -80,16 +77,13 @@ public void testCryptProtectUnprotectDataWithEntropy() {
assertEquals("description", pDescription.getValue().getWideString(0));
assertEquals("hello world", pDataDecrypted.pbData.getString(0));
} finally {
assertEquals("Failed to free descrypted data",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataDecrypted.pbData));
Kernel32Util.freeLocalMemory(pDataDecrypted.pbData);
}
} finally {
assertEquals("Failed to free description",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDescription.getValue()));
Kernel32Util.freeLocalMemory(pDescription.getValue());
}
} finally {
assertEquals("Failed to free encrypted data",
WinError.ERROR_SUCCESS, Kernel32Util.freeLocalMemory(pDataEncrypted.pbData));
Kernel32Util.freeLocalMemory(pDataEncrypted.pbData);
}
}

Expand Down
26 changes: 26 additions & 0 deletions contrib/platform/test/com/sun/jna/platform/win32/Kernel32UtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
import java.util.List;
import java.util.Map;

import com.sun.jna.Pointer;
import com.sun.jna.platform.win32.Tlhelp32.MODULEENTRY32W;
import com.sun.jna.platform.win32.WinNT.HANDLE;
import com.sun.jna.platform.win32.WinNT.HRESULT;
import com.sun.jna.platform.win32.WinNT.LARGE_INTEGER;

import junit.framework.TestCase;
Expand Down Expand Up @@ -91,6 +93,30 @@ private static String formatBytes(long bytes) {
}
}

public void testFreeLocalMemory() {
try {
Pointer ptr = new Pointer(0xFFFFFFFFFFFFFFFFL);
Kernel32Util.freeLocalMemory(ptr);
fail("Unexpected success to free bad local memory");
} catch(Win32Exception e) {
HRESULT hr = e.getHR();
int code = W32Errors.HRESULT_CODE(hr.intValue());
assertEquals("Mismatched failure reason code", WinError.ERROR_INVALID_HANDLE, code);
}
}

public void testFreeGlobalMemory() {
try {
Pointer ptr = new Pointer(0xFFFFFFFFFFFFFFFFL);
Kernel32Util.freeGlobalMemory(ptr);
fail("Unexpected success to free bad global memory");
} catch(Win32Exception e) {
HRESULT hr = e.getHR();
int code = W32Errors.HRESULT_CODE(hr.intValue());
assertEquals("Mismatched failure reason code", WinError.ERROR_INVALID_HANDLE, code);
}
}

public void testGetComputerName() {
assertTrue(Kernel32Util.getComputerName().length() > 0);
}
Expand Down
3 changes: 1 addition & 2 deletions contrib/platform/test/com/sun/jna/platform/win32/VersionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ public void testGetFileVersion() {
assertTrue("dwFileVersionLS", fixedFileInfo.dwFileVersionLS.longValue() > 0);
assertTrue("dwFileVersionMS", fixedFileInfo.dwFileVersionMS.longValue() > 0);
} finally {
assertEquals("Failed to free buffer",
WinError.ERROR_SUCCESS, Kernel32Util.freeGlobalMemory(buffer));
Kernel32Util.freeGlobalMemory(buffer);
}
}
}

0 comments on commit 0acce1f

Please sign in to comment.