diff --git a/docs/ReleaseNotes.html b/docs/ReleaseNotes.html index 9197973..b8fbba9 100644 --- a/docs/ReleaseNotes.html +++ b/docs/ReleaseNotes.html @@ -21,6 +21,14 @@ Bugs +   +   + + + 1775889 + (M4) Fixed leak setString(int[],value) and other setString() methods + +     @@ -29,7 +37,8 @@ 1709841 - (M1) Compiled with Visual Studio 2005. + (M1) Compiled with Visual Studio 2005. Jacob now requires + 2005 or later libraries. See the UsingJacob.html file for impact this has on NT, 2000 and Server 2003 users. @@ -40,7 +49,6 @@ backwards compatible by default. -     diff --git a/jni/SafeArray.cpp b/jni/SafeArray.cpp index 9b191c6..0f7599a 100644 --- a/jni/SafeArray.cpp +++ b/jni/SafeArray.cpp @@ -92,7 +92,7 @@ SAFEARRAY *copySA(SAFEARRAY *psa) VARIANT v1, v2; VariantInit(&v1); - VariantInit(&v2); + VariantInit(&v2); V_VT(&v1) = VT_ARRAY | vt; V_ARRAY(&v1) = psa; VariantCopy(&v2, &v1); @@ -142,11 +142,11 @@ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_init // build the sa according to params if ( nDims > 0 ) { - SAFEARRAY *sa = makeArray(vt, nDims, lbounds, celems); - env->ReleaseIntArrayElements(lb, lbounds, 0); - env->ReleaseIntArrayElements(cel, celems, 0); - jclass clazz = env->GetObjectClass(_this); - setSA(env, _this, sa, 0); + SAFEARRAY *sa = makeArray(vt, nDims, lbounds, celems); + env->ReleaseIntArrayElements(lb, lbounds, 0); + env->ReleaseIntArrayElements(cel, celems, 0); + jclass clazz = env->GetObjectClass(_this); + setSA(env, _this, sa, 0); } } @@ -565,7 +565,7 @@ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_fromStringArray // GetStringUTFChars() replaced with GetStringChars() // (Variant modified in previous report) const jchar *str = env->GetStringChars(s, NULL); - CComBSTR bs((LPCOLESTR)str); // SR cast SF 1689061 + CComBSTR bs((LPCOLESTR)str); // SR cast SF 1689061 V_VT(&v) = VT_BSTR; V_BSTR(&v) = bs.Copy(); long x = i; @@ -1629,7 +1629,7 @@ JNIEXPORT jstring JNICALL Java_com_jacob_com_SafeArray_getString__I BSTR bs = V_BSTR(&v); jstring js = env->NewString((jchar*)bs, SysStringLen(bs)); // SR cast SF 1689061 // jacob report 1224219 - // SafeArrayGetElement memory must be freed http://www.canaimasoft.com/f90VB/OnlineManuals/Reference/TH_31.htm + // SafeArrayGetElement memory must be freed http://www.canaimasoft.com/f90VB/OnlineManuals/Reference/TH_31.htm VariantClear(&v); return js; } else if (vt == VT_BSTR) { @@ -1637,8 +1637,8 @@ JNIEXPORT jstring JNICALL Java_com_jacob_com_SafeArray_getString__I SafeArrayGetElement(psa, &idx, &bs); jstring js = env->NewString((jchar*)bs, SysStringLen(bs)); // SR cast SF 1689061 // jacob report 1224219 - // SafeArrayGetElement memory must be freed http://www.canaimasoft.com/f90VB/OnlineManuals/Reference/TH_31.htm - if (bs) SysFreeString(bs); + // SafeArrayGetElement memory must be freed http://www.canaimasoft.com/f90VB/OnlineManuals/Reference/TH_31.htm + if (bs) SysFreeString(bs); return js; } ThrowComFail(env, "safearray cannot get string", 0); @@ -2456,10 +2456,12 @@ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_setVariants } /* PLEASE NOTE THE LINE: -jint *jIndices = env->GetIntArrayElements(indices, NULL); -which I added to replace "long idx[2] = {i,j};" from the 2D case. -Not sure if this is correct. Also, check that I am doing the null test and -dimension test correctly. + jint *jIndices = env->GetIntArrayElements(indices, NULL); + which I added to replace "long idx[2] = {i,j};" from the 2D case. + Not sure if this is correct. Also, check that I am doing the null test and + dimension test correctly. + Would really like to call env->ReleaseIntArrayElements(indices, jIndices, NULL); + in here but I can't get it to work */ #define GETNDCODE(varType, varAccess, jtyp) \ @@ -2500,10 +2502,12 @@ dimension test correctly. //--------------------------------- /* PLEASE NOTE THE LINE: -jint *jIndices = env->GetIntArrayElements(indices, NULL); -which I added to replace "long idx[2] = {i,j};" from the 2D case. -Not sure if this is correct. Also, check that I am doing the null test and -dimension test correctly. + jint *jIndices = env->GetIntArrayElements(indices, NULL); + which I added to replace "long idx[2] = {i,j};" from the 2D case. + Not sure if this is correct. Also, check that I am doing the null test and + dimension test correctly. + Would really like to call env->ReleaseIntArrayElements(indices, jIndices, NULL); + in here but I can't get it to work */ #define SETNDCODE(varType, varAccess) \ @@ -2586,6 +2590,7 @@ JNIEXPORT jobject JNICALL Java_com_jacob_com_SafeArray_getVariant___3I } else { ThrowComFail(env, "safearray type is not variant/dispatch", -1); } + env->ReleaseIntArrayElements(indices, jIndices, NULL); return newVariant; } @@ -2633,6 +2638,7 @@ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_setVariant___3ILcom_jacob_co } else { ThrowComFail(env, "safearray type is not variant/dispatch", -1); } + env->ReleaseIntArrayElements(indices, jIndices, NULL); } /* @@ -2756,6 +2762,7 @@ JNIEXPORT jstring JNICALL Java_com_jacob_com_SafeArray_getString___3I VARIANT v; VariantInit(&v); SafeArrayGetElement(psa, jIndices, &v); + env->ReleaseIntArrayElements(indices, jIndices, NULL); if (FAILED(VariantChangeType(&v, &v, 0, VT_BSTR))) { return NULL; } @@ -2765,18 +2772,22 @@ JNIEXPORT jstring JNICALL Java_com_jacob_com_SafeArray_getString___3I } else if (vt == VT_BSTR) { BSTR bs = NULL; SafeArrayGetElement(psa, jIndices, &bs); + env->ReleaseIntArrayElements(indices, jIndices, NULL); jstring js = env->NewString((jchar*)bs, SysStringLen(bs)); // SR cast SF 1689061 return js; + } else { + ThrowComFail(env, "safearray cannot get string", 0); + env->ReleaseIntArrayElements(indices, jIndices, NULL); + return NULL; } - ThrowComFail(env, "safearray cannot get string", 0); - return NULL; } /* * Class: com_jacob_com_SafeArray - * Method: setString - * Signature: ([ILjava/lang/String;)V + * Method: setStringjIndices, NULL);ILjava/lang/String;)V + * + * This method has a leak in it somewhere. */ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_setString___3ILjava_lang_String_2 (JNIEnv *env, jobject _this, jintArray indices, jstring s) @@ -2788,6 +2799,7 @@ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_setString___3ILjava_lang_Str } // ??? Not sure if this is correct type for call to SafeArrayGetElement() + // could have individually retrieved the indicies jint *jIndices = env->GetIntArrayElements(indices, NULL); if (!jIndices) { @@ -2819,6 +2831,8 @@ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_setString___3ILjava_lang_Str } else { ThrowComFail(env, "safearray cannot set string", 0); } + // need to unpin the copied array SF: 1775889 + env->ReleaseIntArrayElements(indices, jIndices, NULL); } /* @@ -2898,6 +2912,7 @@ JNIEXPORT jboolean JNICALL Java_com_jacob_com_SafeArray_getBoolean___3I VARIANT v; VariantInit(&v); SafeArrayGetElement(sa, jIndices, (void*) &v); + env->ReleaseIntArrayElements(indices, jIndices, NULL); if (FAILED(VariantChangeType(&v, &v, 0, VT_BOOL))) { ThrowComFail(env, "safearray change type failed", -1); return NULL; @@ -2907,9 +2922,11 @@ JNIEXPORT jboolean JNICALL Java_com_jacob_com_SafeArray_getBoolean___3I } else if (vt == VT_BOOL) { VARIANT_BOOL vb; SafeArrayGetElement(sa, jIndices, (void*) &vb); + env->ReleaseIntArrayElements(indices, jIndices, NULL); jboolean jb = vb == VARIANT_TRUE ? JNI_TRUE: JNI_FALSE; return jb; } else { + env->ReleaseIntArrayElements(indices, jIndices, NULL); return NULL; } } @@ -2954,6 +2971,7 @@ JNIEXPORT void JNICALL Java_com_jacob_com_SafeArray_setBoolean___3IZ } else { ThrowComFail(env, "safearray type mismatch", -1); } + env->ReleaseIntArrayElements(indices, jIndices, NULL); } diff --git a/src/com/jacob/com/SafeArray.java b/src/com/jacob/com/SafeArray.java index 8ea4e15..83839c6 100644 --- a/src/com/jacob/com/SafeArray.java +++ b/src/com/jacob/com/SafeArray.java @@ -484,14 +484,14 @@ public class SafeArray extends JacobObject { public native String getString(int sa_idx1, int sa_idx2); /** - * string access + * puts a string into an element in a single dimensional safe array * @param sa_idx * @param c */ public native void setString(int sa_idx, String c); /** - * string access + * puts a string into an element in a two dimensional array. * @param sa_idx1 * @param sa_idx2 * @param c diff --git a/unittest/com/jacob/test/BaseTestCase.java b/unittest/com/jacob/test/BaseTestCase.java index 60823ea..cbd2cd7 100644 --- a/unittest/com/jacob/test/BaseTestCase.java +++ b/unittest/com/jacob/test/BaseTestCase.java @@ -70,19 +70,20 @@ public class BaseTestCase extends TestCase { // change all "." to ^ for later conversion to "/" so we can append resource names with "." classPackageName = classPackageName.replace('.', '^'); - System.out.println("classPackageName:" + classPackageName); - - String fullPathToResource = (classPackageName.length() > 0 ? classPackageName - + "^" - : "") - + resourceName; + System.out.println("classPackageName: " + classPackageName); + String fullPathToResource; + if (classPackageName.length()> 0){ + fullPathToResource = classPackageName + "^" + resourceName; + } else { + fullPathToResource = resourceName; + } fullPathToResource = fullPathToResource.replace('^', '/'); - System.out.println("fullPathToResource:" + fullPathToResource); + System.out.println("fullPathToResource: " + fullPathToResource); URL urlToLibrary = classInSamePackageAsResource.getClassLoader().getResource(fullPathToResource); - assertNotNull(urlToLibrary); + assertNotNull("URL to resource "+resourceName+" should not be null",urlToLibrary); String fullPathToResourceAsFile = urlToLibrary.getFile(); System.out.println("url to library: "+urlToLibrary); System.out.println("fullPathToResourceAsFile: "+fullPathToResourceAsFile); diff --git a/unittest/com/jacob/test/safearray/SafeArrayLeak.java b/unittest/com/jacob/test/safearray/SafeArrayLeak.java new file mode 100644 index 0000000..deabe04 --- /dev/null +++ b/unittest/com/jacob/test/safearray/SafeArrayLeak.java @@ -0,0 +1,193 @@ +package com.jacob.test.safearray; + +import com.jacob.activeX.ActiveXComponent; +import com.jacob.com.ComThread; +import com.jacob.com.Dispatch; +import com.jacob.com.JacobObject; +import com.jacob.com.SafeArray; +import com.jacob.com.Variant; +import com.jacob.test.BaseTestCase; + +/** + * This test program demonstrates a weak in the setString(int[],String) method in SafeArray. + * To see the leak: + * + * You should see the Page File Usage History graph rise at te end of every cycle. + * Running the same program with setString(r,c,String) does not show the same symptoms + */ +public class SafeArrayLeak extends BaseTestCase { + + /** + * ---------------------------------------------------------------------------------------------------------------------------- + * + * ---------------------------------------------------------------------------------------------------------------------------- + */ + public void testLeakWithSetString() { + + ActiveXComponent xl = null; + Dispatch workbooks = null; + Dispatch workbook = null; + Dispatch workSheets = null; + Dispatch sheet = null; + Dispatch tabCells = null; + SafeArray sa = null; + + //-Dcom.jacob.autogc=true + System.out.println("Jacob version: "+JacobObject.getBuildVersion()); + + for(int t = 0; t < 10; t++) { + // look at a large range of cells + String position = "A7:DM8934"; + + try { + xl = new ActiveXComponent("Excel.Application"); + System.out.println("Excel version=" + xl.getProperty("Version")); + + xl.setProperty("Visible", new Variant(false)); + workbooks = xl.getProperty("Workbooks").toDispatch(); + + workbook = Dispatch.get(workbooks,"Add").toDispatch(); + + workSheets = Dispatch.get(workbook, "Worksheets").toDispatch(); + + sheet = Dispatch.get(workbook, "ActiveSheet").toDispatch(); + // grab the whole range specified above. + tabCells = Dispatch.invoke(sheet, "Range", Dispatch.Get, + new Object[] { position }, + new int[1]).toDispatch(); + + sa = Dispatch.get(tabCells, "Value").toSafeArray(true); + + System.out.println("Ub0=" + sa.getUBound(1)); // nbCol + System.out.println("Ub1=" + sa.getUBound(2)); // nbLgn + + // number of rows + int nbLgn = sa.getUBound(2); + // number of columns + int nbCol = sa.getUBound(1); + + int[] colLgn = new int[] { 0, 0 }; + + // now set a value on every cell in the range we retrieved + for(int i = 1; i <= nbLgn; i++) { + colLgn[1] = i; + + for(int j = 1; j <= nbCol; j++) { + colLgn[0] = j; + // this one works with out a leak 1.13-M3 + //sa.setString(j, i, "test"); + // This one leaks with 1.13-M3 and earlier + sa.setString(colLgn, "test"); + } + } + + Dispatch.put(tabCells, "Value", sa); + + Variant f = new Variant(false); + Dispatch.call(workbook, "Close", f); + System.out.println("Close"); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + + if (sa != null) { + try { + sa.safeRelease(); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + sa = null; + } + } + + if (tabCells != null) { + try { + tabCells.safeRelease(); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + tabCells = null; + } + } + + if (sheet != null) { + try { + sheet.safeRelease(); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + sheet = null; + } + } + + if (workSheets != null) { + try { + workSheets.safeRelease(); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + workSheets = null; + } + } + + if (workbook != null) { + try { + workbook.safeRelease(); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + workbook = null; + } + } + + if (workbooks != null) { + try { + workbooks.safeRelease(); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + workbooks = null; + } + } + + if (xl != null) { + try { + xl.invoke("Quit", new Variant[] {}); + } + catch(Exception e) { + e.printStackTrace(); + } + + try { + xl.safeRelease(); + } + catch(Exception e) { + e.printStackTrace(); + } + finally { + xl = null; + } + } + + ComThread.Release(); + } + } + } +}