Bug 24402 - OLE part have a access violation exception
Summary: OLE part have a access violation exception
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 2.0.1   Edit
Hardware: PC Windows 2000
: P4 enhancement (vote)
Target Milestone: 2.1 M2   Edit
Assignee: Veronika Irvine CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2002-10-04 12:59 EDT by Yang Liu CLA
Modified: 2004-09-24 11:19 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yang Liu CLA 2002-10-04 12:59:22 EDT
First time to report a bug here -- and new to eclipse.

This bug is relating to the OLE part, so only appear on Windows platform.

I am trying to put some ActiveX control in my application, and I need to get a
IDispatch property of an OleAutomation.

I call OleAutomation.getProperty(), this function will then invoke the
underlying IDispatch.invoke(), and later call into Variant.setData() to copy the
result into the Variant structure.

In the Variant.setData() function, there is something as:

		case COM.VT_DISPATCH : {
			int[] ppvObject = new int[1];
			OS.MoveMemory(ppvObject, pData + 8, 4);
			dispatchData = new IDispatch(ppvObject[0]);
			dispatchData.AddRef();
			break;
		}

This is for when the return type is VT_DISPATCH, then create a IDispatch object
and AddRef() to it. Unfortunately, ppvObject[0] can be null. In this case,
AddRef() crash the system.

I believe, this part of code should be:

		case COM.VT_DISPATCH : {
			int[] ppvObject = new int[1];
			OS.MoveMemory(ppvObject, pData + 8, 4);
			if (ppvObject[0]!=null) {
			    dispatchData = new IDispatch(ppvObject[0]);
			    dispatchData.AddRef();
			} else {
			    dispatchData = null;
			}
			break;
		}


BTW I like SWT very much.
Comment 1 Steve Northover CLA 2002-10-07 10:55:27 EDT
VI to investigate and advise.
Comment 2 Veronika Irvine CLA 2002-10-10 10:01:25 EDT
Fixed > 20021010

However, instead of setting the data field to null, I have set the type to 
VT_EMPTY.  I have also added extra checking for VT_EMPTY when requesting the 
Java type from a Variant (could happen in several ways).  Added checking to all 
places where a pointer or object reference is obtained.
Comment 3 Yang Liu CLA 2002-10-10 18:57:33 EDT
But why do changing type to VARIANT_EMPTY?

And if there is good reason to change to VARIANT_EMPTY, I would rather the 
getAutomation() and getDispatch() to return null, instead of raising an error. 
in this way, I can easily do things as

propertyAutomation = oleAutomationObj.getProperty(dispid).getAutomation();
if (propertyAutomation!=null) {
    dosomething
}

Also, when the type is VARIANT_EMPTY, I would suggest getInt() just return 0; 
getBoolean() return false; getUnknown() return null as getDispatch(). Not 
raising error. This may more confirm to the common COM programming practise.
Comment 4 Veronika Irvine CLA 2002-10-11 09:17:25 EDT
As the javadoc already indicates, you need to handle the SWTError 
ERROR_CANNOT_CHANGE_VARIANT_TYPE since the application may have given a variant 
of one type that cannot be coerced into the type you have requested.

To return 0 for an int when the type is VT_EMPTY is misleading.  The 
application may be using VT_EMPTY as a way of signalling that an error has 
occured (they should be using a return type but clearly not all applications do 
what they should do). To interpret this as 0 when 0 is a prefectly valid answer 
would be incorrect.

COM provides an error code when you try coerce a data type into something 
else.  The OO way of dealing with error codes is to throw exceptions and that 
is why I am throwing the SWTError.
Comment 5 Yang Liu CLA 2002-10-11 11:54:26 EDT
But I would still suggest that return null for the

propertyAutomation = oleAutomationObj.getProperty(dispid).getAutomation();

case.

I think you needn't change the type to VARIANT_EMPTY when the IDispatch is null.
This will make the programmer's life much easier, and confirm to COM.

I care less about getInt(), since most the case, the COM server will return an
int value, instead of VARIANT_EMPTY.
Comment 6 Yang Liu CLA 2002-10-14 05:34:11 EDT
I reopened this bug. Since I do not like the current API. I still want to
getOleAutomation to return null instead of raise error when the IDispatch
property is null.

Changed severity to enhancement.
Comment 7 Veronika Irvine CLA 2002-10-15 08:40:20 EDT
I would rather be consistent across all types of Variant.  I do not want to 
handle VT_DISPATCH different from any other case.

I understand that null makes your code easier in this one case.  However, in 
theory you should still handle the exception because some future implementation 
of the control could return a value that can not be coerced into an IDispatch 
object and then you will still get the error.

Setting dispatchData to null would also impact getAutomation.
Comment 8 Yang Liu CLA 2002-10-15 19:42:03 EDT
To me, the consistent behavior should be:
Variant never change the VT_TYPE by its own. That is, if the underlying Windows 
API return a VARIANT of type VT_DISPATCH, then the corresponding java Variant 
should also be VT_DISPATCH. Then getDispatch() will return the same thing as 
you get the dispatch value from the underlying Windows VARIANT -- that 
is "null".

For getAutomation() and getUnknown(), they should all behave the same way, that 
is return null when the Variant has the same type required and value is null.

This behavior confirm to all other COM/OLE implementations, as far as I know, 
the native Windows COM API behave this way; Microsoft's Java VM behave this 
way; JACOB also behave this way.
Comment 9 Yang Liu CLA 2002-10-22 03:55:45 EDT
really inconvenient to always catch the type error, and make the code very ugly.
So I reopen the bug.
Comment 10 Veronika Irvine CLA 2002-10-22 08:34:30 EDT
It is not neccessary to always catch the error.  You can check the type 
(Variant.getType) for VT_EMPTY before calling Variant.getDispatch.
Comment 11 Yang Liu CLA 2002-10-22 20:24:48 EDT
I still think that is an unnecessary step. The behavior of returning null is
consistent with other system, and easier to understand. Why should Variant
automatically change the type?
Comment 12 Veronika Irvine CLA 2004-09-24 11:19:57 EDT
This problem has been fixed.