Commit 4dca743c authored by George Nash's avatar George Nash

Overwrite the SWIG generated freeEndpoint code to avoid double free memory error

The OCEndpointUtil.freeEndpoint method will free the memory associated with an
endpoint. If the Java code thinks that it is responsible for freeing the
memory it will also free the endpoint when the GC is run. This results in
freeing memory that was already freed and may be in use by other memory.

This updates the JNI code to check if Java thinks it is responsible for the
memory associated with the endpoint being freews.  If so it will update
the OCEndpoint in question so it no longer thinks its responsible for the
native memory associated with the OCEndpoint. I addition the copy of the
C pointer held in the OCEndpoint class is set to 0 (i.e. NULL) to avoid
having a floating pointer that could cause other problems.
Signed-off-by: George Nash's avatarGeorge Nash <george.nash@intel.com>
parent 897060fe
Pipeline #682 passed with stage
in 11 minutes and 50 seconds
......@@ -25,6 +25,7 @@ public class OCEndpointTest {
assertEquals("/a/light", uri[0]);
assertArrayEquals(new short[]{10, 211, 55, 3}, ep.getAddr().getIpv4().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// IPV6
try {
......@@ -42,6 +43,7 @@ public class OCEndpointTest {
assertArrayEquals(new short[]{0xff, 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x58},
ep.getAddr().getIpv6().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// IPV6 with uri
try {
......@@ -59,6 +61,7 @@ public class OCEndpointTest {
assertArrayEquals(new short[]{0xff, 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x58},
ep.getAddr().getIpv6().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// IPV6 with port and uri
try {
......@@ -76,6 +79,7 @@ public class OCEndpointTest {
assertArrayEquals(new short[]{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x12},
ep.getAddr().getIpv6().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
}
// The tests will fail on Windows. It does not yet support dns lookup.
......@@ -98,6 +102,7 @@ public class OCEndpointTest {
assertEquals(5683, ep.getAddr().getIpv4().getPort());
assertNull(uri[0]);
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// dns lookup with uri
try {
......@@ -114,6 +119,7 @@ public class OCEndpointTest {
assertEquals(5683, ep.getAddr().getIpv4().getPort());
assertEquals("/alpha", uri[0]);
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// dns lookup with port and uri
try {
......@@ -130,6 +136,7 @@ public class OCEndpointTest {
assertEquals(3456, ep.getAddr().getIpv4().getPort());
assertEquals("/alpha", uri[0]);
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
}
// The tests will fail on Windows. It does not yet support tcp.
......@@ -152,6 +159,7 @@ public class OCEndpointTest {
assertEquals("/a/light", uri[0]);
assertArrayEquals(new short[]{10, 211, 55, 3}, ep.getAddr().getIpv4().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// IPv4 over tcp and port
try {
......@@ -168,6 +176,7 @@ public class OCEndpointTest {
assertNull(uri[0]);
assertArrayEquals(new short[]{1, 2, 3, 4}, ep.getAddr().getIpv4().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// IPv6 over tcp
try {
......@@ -185,6 +194,7 @@ public class OCEndpointTest {
assertArrayEquals(new short[]{0xff, 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x58},
ep.getAddr().getIpv6().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// IPv6 over tcp with uri
try {
......@@ -202,6 +212,7 @@ public class OCEndpointTest {
assertArrayEquals(new short[]{0xff, 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x58},
ep.getAddr().getIpv6().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
// IPv6 over tcp with port and uri
try {
......@@ -219,6 +230,7 @@ public class OCEndpointTest {
assertArrayEquals(new short[]{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x12},
ep.getAddr().getIpv6().getAddress());
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
}
// The tests will fail on Windows. It does not yet support tcp or dns lookup.
......@@ -241,6 +253,7 @@ public class OCEndpointTest {
assertEquals(3456, ep.getAddr().getIpv4().getPort());
assertNull(uri[0]);
OCEndpointUtil.freeEndpoint(ep);
assertEquals(0, OCEndpoint.getCPtr(ep));
}
@Test
......
......@@ -55,13 +55,51 @@
// look into exposing oc_make_ipv4_endpoint and oc_make_ipv6_endpoint
%rename(newEndpoint) oc_new_endpoint;
%ignore oc_free_endpoint;
%rename(freeEndpoint) jni_free_endpoint;
// tell swig to use our JNI code not to generate its own.
%native (freeEndpoint) void freeEndpoint(oc_endpoint_t *endpoint);
%inline %{
void jni_free_endpoint(oc_endpoint_t *endpoint) {
oc_free_endpoint(endpoint);
endpoint = NULL;
}
%}
%{
/*
* Hand rolled JNI code. Is here to prevent double freeing of memory.
* If `freeEndpoint` is called then the developer is explicitly taking ownership
* of the `OCEndpoint`. If `swigCMemOwn` is `true` it must be changed to false
* to prevent the Java GC from trying to free the same block of memory a second
* time.
*
* Since this is freeing memory we also set the swigCPtr to null to instantly
* cause code failures should the developer try and use the Java OCEndpoint that
* they just freed. In JUnit value of the `swigCPtr` can be checked to verify the
* operation of this code. We can not check `swigCMemOwn` directly because it is
* a private member variable with not get method to access it.
*/
SWIGEXPORT void JNICALL Java_org_iotivity_OCEndpointUtilJNI_freeEndpoint(JNIEnv *jenv, jclass jcls, jlong jarg1, jobject jarg1_) {
OC_DBG("JNI: %s\n", __func__);
oc_endpoint_t *arg1 = NULL;
(void) jcls;
jboolean jswigCMemOwn = false;
jfieldID swigCMemOwn_fid = (*jenv)->GetFieldID(jenv, cls_OCEndpoint, "swigCMemOwn", "Z");
if (swigCMemOwn_fid != 0) {
jswigCMemOwn = (*jenv)->GetBooleanField(jenv, jarg1_, swigCMemOwn_fid);
if (jswigCMemOwn) {
(*jenv)->SetBooleanField(jenv, jarg1_, swigCMemOwn_fid, false);
}
}
arg1 = (oc_endpoint_t *)jarg1;
jni_free_endpoint(arg1);
jfieldID swigCPtr_fid = (*jenv)->GetFieldID(jenv, cls_OCEndpoint, "swigCPtr", "J");
if (swigCPtr_fid != 0) {
(*jenv)->SetLongField(jenv, jarg1_, swigCPtr_fid, 0);
}
}
%}
%ignore oc_endpoint_set_di;
%exception oc_endpoint_set_di {
/* The `oc_endpoint_t *endpoint` parameter is jarg1, the name is generated by SWIG. */
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment