Bug 121764 - correcting AddRef/Release reference counting

2 files changed, 90 insertions, 15 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT Accessibility/win32/org/eclipse/swt/accessibility/Accessible.java b/bundles/org.eclipse.swt/Eclipse SWT Accessibility/win32/org/eclipse/swt/accessibility/Accessible.java
index 70ea48c83b..ece53f2420 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT Accessibility/win32/org/eclipse/swt/accessibility/Accessible.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT Accessibility/win32/org/eclipse/swt/accessibility/Accessible.java
@@ -52,12 +52,13 @@ public class Accessible {
 	Accessible(Control control) {
 		this.control = control;
 		int[] ppvObject = new int[1];
+		/* CreateStdAccessibleObject([in] hwnd, [in] idObject, [in] riidInterface, [out] ppvObject).
+		 * AddRef has already been called on ppvObject by the callee and must be released by the caller.
+		 */
 		int result = COM.CreateStdAccessibleObject(control.handle, COM.OBJID_CLIENT, COM.IIDIAccessible, ppvObject);
-		if (ppvObject[0] == 0) return;
 		if (result != COM.S_OK) OLE.error(OLE.ERROR_CANNOT_CREATE_OBJECT, result);
 		iaccessible = new IAccessible(ppvObject[0]);
-		iaccessible.AddRef();
-		
+	
 		objIAccessible = new COMObject(new int[] {2,0,0,1,3,5,8,1,1,5,5,5,5,5,5,5,6,5,1,1,5,5,8,6,3,4,5,5}) {
 			public int method0(int[] args) {return QueryInterface(args[0], args[1]);}
 			public int method1(int[] args) {return AddRef();}
@@ -223,8 +224,9 @@ public class Accessible {
 	 * </p>
 	 */
 	public void internal_dispose_Accessible() {
-		if (iaccessible != null)
+		if (iaccessible != null) {
 			iaccessible.Release();
+		}
 		iaccessible = null;
 		Release();
 	}
@@ -242,6 +244,10 @@ public class Accessible {
 	public int internal_WM_GETOBJECT (int wParam, int lParam) {
 		if (objIAccessible == null) return 0;
 		if (lParam == COM.OBJID_CLIENT) {
+			/* LresultFromObject([in] riid, [in] wParam, [in] pAcc)
+			 * The argument pAcc is owned by the caller so reference count does not
+			 * need to be incremented.
+			 */
 			return COM.LresultFromObject(COM.IIDIAccessible, wParam, objIAccessible.getAddress());
 		}
 		return 0;
@@ -414,39 +420,43 @@ public class Accessible {
 		// not an MSAA event
 	}
 	
-	int QueryInterface(int arg1, int arg2) {
+	/* QueryInterface([in] iid, [out] ppvObject)
+	 * Ownership of ppvObject transfers from callee to caller so reference count on ppvObject 
+	 * must be incremented before returning.  Caller is responsible for releasing ppvObject.
+	 */
+	int QueryInterface(int iid, int ppvObject) {
 		if (iaccessible == null) return COM.CO_E_OBJNOTCONNECTED;
 		GUID guid = new GUID();
-		COM.MoveMemory(guid, arg1, GUID.sizeof);
+		COM.MoveMemory(guid, iid, GUID.sizeof);
 
 		if (COM.IsEqualGUID(guid, COM.IIDIUnknown)) {
-			COM.MoveMemory(arg2, new int[] { objIAccessible.getAddress()}, 4);
+			COM.MoveMemory(ppvObject, new int[] { objIAccessible.getAddress()}, 4);
 			AddRef();
 			return COM.S_OK;
 		}
 
 		if (COM.IsEqualGUID(guid, COM.IIDIDispatch)) {
-			COM.MoveMemory(arg2, new int[] { objIAccessible.getAddress()}, 4);
+			COM.MoveMemory(ppvObject, new int[] { objIAccessible.getAddress()}, 4);
 			AddRef();
 			return COM.S_OK;
 		}
 
 		if (COM.IsEqualGUID(guid, COM.IIDIAccessible)) {
-			COM.MoveMemory(arg2, new int[] { objIAccessible.getAddress()}, 4);
+			COM.MoveMemory(ppvObject, new int[] { objIAccessible.getAddress()}, 4);
 			AddRef();
 			return COM.S_OK;
 		}
 
 		if (COM.IsEqualGUID(guid, COM.IIDIEnumVARIANT)) {
-			COM.MoveMemory(arg2, new int[] { objIEnumVARIANT.getAddress()}, 4);
+			COM.MoveMemory(ppvObject, new int[] { objIEnumVARIANT.getAddress()}, 4);
 			AddRef();
 			enumIndex = 0;
 			return COM.S_OK;
 		}
 
-		int[] ppvObject = new int[1];
-		int result = iaccessible.QueryInterface(guid, ppvObject);
-		COM.MoveMemory(arg2, ppvObject, 4);
+		int[] ppv = new int[1];
+		int result = iaccessible.QueryInterface(guid, ppv);
+		COM.MoveMemory(ppvObject, ppv, 4);
 		return result;
 	}
 
@@ -552,6 +562,10 @@ public class Accessible {
 		return code;
 	}
 
+	/* get_accChild([in] varChild, [out] ppdispChild)
+	 * Ownership of ppdispChild transfers from callee to caller so reference count on ppdispChild 
+	 * must be incremented before returning.  The caller is responsible for releasing ppdispChild.
+	 */
 	int get_accChild(int varChild_vt, int varChild_reserved1, int varChild_lVal, int varChild_reserved2, int ppdispChild) {
 		if (iaccessible == null) return COM.CO_E_OBJNOTCONNECTED;
 		if ((varChild_vt & 0xFFFF) != COM.VT_I4) return COM.E_INVALIDARG;
@@ -569,6 +583,7 @@ public class Accessible {
 		}
 		Accessible accessible = event.accessible;
 		if (accessible != null) {
+			accessible.AddRef();
 			COM.MoveMemory(ppdispChild, new int[] { accessible.objIAccessible.getAddress() }, 4);
 			return COM.S_OK;
 		}
@@ -697,6 +712,10 @@ public class Accessible {
 		return COM.S_OK;
 	}
 	
+	/* get_accFocus([out] int pvarChild)
+	 * Ownership of pvarChild transfers from callee to caller so reference count on pvarChild 
+	 * must be incremented before returning.  The caller is responsible for releasing pvarChild.
+	 */
 	int get_accFocus(int pvarChild) {
 		if (iaccessible == null) return COM.CO_E_OBJNOTCONNECTED;
 
@@ -722,6 +741,7 @@ public class Accessible {
 		}
 		Accessible accessible = event.accessible;
 		if (accessible != null) {
+			accessible.AddRef();
 			COM.MoveMemory(pvarChild, new short[] { COM.VT_DISPATCH }, 2);
 			COM.MoveMemory(pvarChild + 8, new int[] { accessible.objIAccessible.getAddress() }, 4);
 			return COM.S_OK;
@@ -732,6 +752,7 @@ public class Accessible {
 			return COM.S_FALSE;
 		}
 		if (childID == ACC.CHILDID_SELF) {
+			AddRef();
 			COM.MoveMemory(pvarChild, new short[] { COM.VT_DISPATCH }, 2);
 			COM.MoveMemory(pvarChild + 8, new int[] { objIAccessible.getAddress() }, 4);
 			return COM.S_OK;
@@ -851,6 +872,10 @@ public class Accessible {
 		return COM.S_OK;
 	}
 	
+	/* get_accParent([out] ppdispParent)
+	 * Ownership of ppdispParent transfers from callee to caller so reference count on ppdispParent 
+	 * must be incremented before returning.  The caller is responsible for releasing ppdispParent.
+	 */
 	int get_accParent(int ppdispParent) {
 		if (iaccessible == null) return COM.CO_E_OBJNOTCONNECTED;
 		// Currently, we don't let the application override this. Forward to the proxy.
@@ -898,6 +923,10 @@ public class Accessible {
 		return COM.S_OK;
 	}
 	
+	/* get_accSelection([out] pvarChildren)
+	 * Ownership of pvarChildren transfers from callee to caller so reference count on pvarChildren 
+	 * must be incremented before returning.  The caller is responsible for releasing pvarChildren.
+	 */
 	int get_accSelection(int pvarChildren) {
 		if (iaccessible == null) return COM.CO_E_OBJNOTCONNECTED;
 
@@ -926,6 +955,7 @@ public class Accessible {
 		}
 		Accessible accessible = event.accessible;
 		if (accessible != null) {
+			accessible.AddRef();
 			COM.MoveMemory(pvarChildren, new short[] { COM.VT_DISPATCH }, 2);
 			COM.MoveMemory(pvarChildren + 8, new int[] { accessible.objIAccessible.getAddress() }, 4);
 			return COM.S_OK;
@@ -936,12 +966,14 @@ public class Accessible {
 			return COM.S_FALSE;
 		}
 		if (childID == ACC.CHILDID_MULTIPLE) {
+			AddRef();
 			COM.MoveMemory(pvarChildren, new short[] { COM.VT_UNKNOWN }, 2);
 			/* Should return an IEnumVARIANT for this... so the next line is wrong... need better API here... */
 			COM.MoveMemory(pvarChildren + 8, new int[] { objIAccessible.getAddress() }, 4);
 			return COM.S_OK;
 		}
 		if (childID == ACC.CHILDID_SELF) {
+			AddRef();
 			COM.MoveMemory(pvarChildren, new short[] { COM.VT_DISPATCH }, 2);
 			COM.MoveMemory(pvarChildren + 8, new int[] { objIAccessible.getAddress() }, 4);
 			return COM.S_OK;
@@ -1068,6 +1100,11 @@ public class Accessible {
 	 * The number of elements actually retrieved is returned in pceltFetched 
 	 * (unless the caller passed in NULL for that parameter).
 	 */
+	 
+	 /* Next([in] celt, [out] rgvar, [in, out] pceltFetched)
+	 * Ownership of rgvar transfers from callee to caller so reference count on rgvar 
+	 * must be incremented before returning.  The caller is responsible for releasing rgvar.
+	 */
 	int Next(int celt, int rgvar, int pceltFetched) {
 		/* If there are no listeners, query the proxy for
 		 * its IEnumVariant, and get the Next items from it.
@@ -1079,6 +1116,7 @@ public class Accessible {
 			IEnumVARIANT ienumvariant = new IEnumVARIANT(ppvObject[0]);
 			int[] celtFetched = new int[1];
 			code = ienumvariant.Next(celt, rgvar, celtFetched);
+			ienumvariant.Release();
 			COM.MoveMemory(pceltFetched, celtFetched, 4);
 			return code;
 		}
@@ -1119,9 +1157,10 @@ public class Accessible {
 					COM.MoveMemory(rgvar + i * 16, new short[] { COM.VT_I4 }, 2);
 					COM.MoveMemory(rgvar + i * 16 + 8, new int[] { item }, 4);
 				} else {
-					int address = ((Accessible) nextItem).objIAccessible.getAddress();
+					Accessible accessible = (Accessible) nextItem;
+					accessible.AddRef();
 					COM.MoveMemory(rgvar + i * 16, new short[] { COM.VT_DISPATCH }, 2);
-					COM.MoveMemory(rgvar + i * 16 + 8, new int[] { address }, 4);
+					COM.MoveMemory(rgvar + i * 16 + 8, new int[] { accessible.objIAccessible.getAddress() }, 4);
 				}
 			}
 			if (pceltFetched != 0)
@@ -1145,6 +1184,7 @@ public class Accessible {
 			if (code != COM.S_OK) return code;
 			IEnumVARIANT ienumvariant = new IEnumVARIANT(ppvObject[0]);
 			code = ienumvariant.Skip(celt);
+			ienumvariant.Release();
 			return code;
 		}
 
@@ -1168,6 +1208,7 @@ public class Accessible {
 			if (code != COM.S_OK) return code;
 			IEnumVARIANT ienumvariant = new IEnumVARIANT(ppvObject[0]);
 			code = ienumvariant.Reset();
+			ienumvariant.Release();
 			return code;
 		}
 		
diff --git a/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/win32/org/eclipse/swt/dnd/Clipboard.java b/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/win32/org/eclipse/swt/dnd/Clipboard.java
index a609ddba05..b2845cf0b9 100755
--- a/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/win32/org/eclipse/swt/dnd/Clipboard.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT Drag and Drop/win32/org/eclipse/swt/dnd/Clipboard.java
@@ -177,7 +177,15 @@ public void clearContents() {
 public void clearContents(int clipboards) {
 	checkWidget();
 	if ((clipboards & DND.CLIPBOARD) != 0) {
+		/* OleIsCurrentClipboard([in] pDataObject)
+		 * The argument pDataObject is owned by the caller so reference count does not
+		 * need to be incremented.
+		 */
 		if (COM.OleIsCurrentClipboard(this.iDataObject.getAddress()) == COM.S_OK) {
+			/* OleSetClipboard([in] pDataObject)
+			 * The argument pDataObject is owned by the caller so reference count does not
+			 * need to be incremented.
+			 */
 			COM.OleSetClipboard(0);
 		}
 	}
@@ -198,6 +206,10 @@ public void clearContents(int clipboards) {
 public void dispose () {
 	if (isDisposed()) return;
 	if (display.getThread() != Thread.currentThread()) DND.error(SWT.ERROR_THREAD_INVALID_ACCESS);
+	/* OleIsCurrentClipboard([in] pDataObject)
+	 * The argument pDataObject is owned by the caller so reference count does not
+	 * need to be incremented.
+	 */
 	if (COM.OleIsCurrentClipboard(this.iDataObject.getAddress()) == COM.S_OK) {
 		COM.OleFlushClipboard();
 	}	
@@ -299,6 +311,9 @@ public Object getContents(Transfer transfer, int clipboards) {
 	*/
 	int[] ppv = new int[1];
 	int retryCount = 0;
+	/* OleGetClipboard([out] ppDataObject).
+	 * AddRef has already been called on ppDataObject by the callee and must be released by the caller.
+	 */
 	int result = COM.OleGetClipboard(ppv);
 	while (result != COM.S_OK && retryCount++ < 10) {
 		try {Thread.sleep(50);} catch (Throwable t) {}
@@ -462,6 +477,10 @@ public void setContents(Object[] data, Transfer[] dataTypes, int clipboards) {
 	if ((clipboards & DND.CLIPBOARD) == 0) return;
 	this.data = data;
 	this.transferAgents = dataTypes;
+	/* OleSetClipboard([in] pDataObject)
+	 * The argument pDataObject is owned by the caller so the reference count does not
+	 * need to be incremented.
+	 */
 	int result = COM.OleSetClipboard(iDataObject.getAddress());
 	
 	/*
@@ -511,6 +530,11 @@ private void disposeCOMInterfaces() {
 		iDataObject.dispose();
 	iDataObject = null;
 }
+/*
+ * EnumFormatEtc([in] dwDirection, [out] ppenumFormatetc)
+ * Ownership of ppenumFormatetc transfers from callee to caller so reference count on ppenumFormatetc 
+ * must be incremented before returning.  Caller is responsible for releasing ppenumFormatetc.
+ */
 private int EnumFormatEtc(int dwDirection, int ppenumFormatetc) {
 	// only allow getting of data - SetData is not currently supported
 	if (dwDirection == COM.DATADIR_SET) return COM.E_NOTIMPL;
@@ -596,6 +620,10 @@ private int QueryGetData(int pFormatetc) {
 	
 	return COM.DV_E_FORMATETC;
 }
+/* QueryInterface([in] iid, [out] ppvObject)
+ * Ownership of ppvObject transfers from callee to caller so reference count on ppvObject 
+ * must be incremented before returning.  Caller is responsible for releasing ppvObject.
+ */
 private int QueryInterface(int riid, int ppvObject) {
 	if (riid == 0 || ppvObject == 0) return COM.E_INVALIDARG;
 	GUID guid = new GUID();
@@ -729,9 +757,15 @@ public String[] getAvailableTypeNames() {
 private FORMATETC[] _getAvailableTypes() {
 	FORMATETC[] types = new FORMATETC[0];
 	int[] ppv = new int[1];
+	/* OleGetClipboard([out] ppDataObject).
+	 * AddRef has already been called on ppDataObject by the callee and must be released by the caller.
+	 */
 	if (COM.OleGetClipboard(ppv) != COM.S_OK) return types;
 	IDataObject dataObject = new IDataObject(ppv[0]);
 	int[] ppFormatetc = new int[1];
+	/* EnumFormatEtc([in] dwDirection, [out] ppenumFormatetc) 
+	 * AddRef has already been called on ppenumFormatetc by the callee and must be released by the caller. 
+	 */
 	int rc = dataObject.EnumFormatEtc(COM.DATADIR_GET, ppFormatetc);
 	dataObject.Release();
 	if (rc != COM.S_OK)return types;