SF 1775889 - There was a like when passing arrays of indexes as part of set or get methods. 1775889 mentioned setString() but there were others also.

docs/ReleaseNotes.html

@@ -21,6 +21,14 @@
     <td width="100%" colspan="2"><b>Bugs</b></td>
   </tr>
   <tr>
+    <td width="13%" valign="top">&nbsp;</td> 
+    <td width="87%" valign="top">&nbsp;</td>
+  </tr>
+  <tr>
+    <td width="13%" valign="top">1775889</td> 
+    <td width="87%" valign="top">(M4) Fixed leak setString(int[],value) and other setString() methods</td>
+  </tr>
+  <tr> 
     <td width="13%" valign="top">&nbsp;</td>
     <td width="87%" valign="top">&nbsp;</td>
   </tr>
@@ -29,7 +37,8 @@
   </tr> 
   <tr>
     <td width="13%" valign="top">1709841 </td>
-    <td width="87%" valign="top">(M1) Compiled with Visual Studio 2005.
+    <td width="87%" valign="top">(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.
     	</td>
   </tr>
@@ -40,7 +49,6 @@
     	backwards compatible by default.
    	</td>
   </tr>
-  <tr>
   <tr> 
     <td width="13%" valign="top">&nbsp;</td>
     <td width="87%" valign="top">&nbsp;</td>

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);
+      SAFEARRAY *sa = makeArray(vt, nDims, lbounds, celems);
-	  env->ReleaseIntArrayElements(lb, lbounds, 0);
+      env->ReleaseIntArrayElements(lb, lbounds, 0);
-	  env->ReleaseIntArrayElements(cel, celems, 0);
+      env->ReleaseIntArrayElements(cel, celems, 0);
-	  jclass clazz = env->GetObjectClass(_this);
+      jclass clazz = env->GetObjectClass(_this);
-	  setSA(env, _this, sa, 0);
+      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
+    // SafeArrayGetElement memory must be freed http://www.canaimasoft.com/f90VB/OnlineManuals/Reference/TH_31.htm
-	if (bs) SysFreeString(bs);
+    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);
+	jint *jIndices = env->GetIntArrayElements(indices, NULL);
-which I added to replace "long idx[2] = {i,j};" from the 2D case.
+	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
+	Not sure if this is correct. Also, check that I am doing the null test and
-dimension test correctly.
+	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);
+	jint *jIndices = env->GetIntArrayElements(indices, NULL);
-which I added to replace "long idx[2] = {i,j};" from the 2D case.
+	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
+	Not sure if this is correct. Also, check that I am doing the null test and
-dimension test correctly.
+	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
+ * Method:    setStringjIndices, NULL);ILjava/lang/String;)V
- * Signature: ([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);
 }
 
 

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

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);
+		System.out.println("classPackageName: " + classPackageName);
-		
+		String fullPathToResource;
-		String fullPathToResource = (classPackageName.length() > 0 ? classPackageName
+		if (classPackageName.length()> 0){
-				+ "^"
+			fullPathToResource = classPackageName + "^" + resourceName;
-				: "")
+		} else {
-				+ resourceName;
+			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);

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:
+ * <ul>
+ * <li>Bring up the windows task manager and click on the performance tab.
+ * <li>Run the test program
+ * </ul>
+ * 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();
+		    }
+	    }
+	}
+}