[Mono-dev] COM Interop Patch

[Zoltan Varga]

                                         Hi,

  Here are my comments:
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
- class.c OK
- loader.c:
	- why is signature->cominterop needed, ie IS_IMPORT (m->klass) can be
used instead ?
- domain.c: OK
- image.c: OK
- metadata.c:
  - The comment of mono_metadata_type_dup_mp is incorrect, also:
  - mono_stats.generics_metadata_size += sizeof (MonoType);
    is not needed
  - this is not needed:
+	r->attrs = original->attrs;
+	r->byref = original->byref;
- metadata-internals.h: OK
- the declaration of mono_metadata_type_dup_mp () should go to
metadata-internals.h to avoid making it public
- object.c OK
- class-internals.h OK
- object-internals.h OK
  - move the '#endregion' in RealProxy.cs to encompass all the fields,  add a
    comment saying that other classes visible to the runtime inherit from this
    class so all new fields should be added between #region-#endregion.
- marshal.c:
  - the comment for cominterop_get_method_interface is wrong
  - in cominterop_get_native_wrapper_adjusted (), the created method is put
    into the cache, but never looked up, so either the lookup is missing, or
    the cache is not really needed.
  - in cominterop_get_native_wrapper (), no need to set save_lmf.
Also, the wrapper type shouldn't be MONO_WRAPPER_MANAGED_TO_NATIVE,
since this calls another managed method, not native code.
  - in cominterop_get_invoke (), sig = signature_no_pinvoke () should be
    moved after the if ((res = mono_marshal_find_in_cached ()))
otherwise it is leaked. This means the if (!sig->hasthis) line needs
to changed as well.
  - in cominterop_get_invoke (), no need to set save_lmf. Also, no
need for emit_thread_interrupt_checkpoint ().
- marshal.h OK
- icall.c: The ComObject icalls should be moved to marshal.c.

- Marshal.cs: I think the [MonoTODO]s should remain, since these methods can
  still throw NotImplementedException ().
- __ComObject.cs: This should use #region-#endregion to mark which fields are
  visible from unmanaged code.
- ActivationService.cs: This could use Type.IsImport.

- need to increase corlib version in appdomain.c and System/Environment.cs.
- it would be nice to add the design comments in your mail to the wiki
page about COM Interop and to the code, like the reasoning behind the
_adjusted methods.
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Other than these, the code look ok to me. Good Work !

             Zoltan

On 7/12/06, Jon Chambers <joncham at gmail.com> wrote:
> Here is another attempt at a COM Interop patch. First, all
> changes/contributions are MIT X11. Now, for a brief overview ;-).
>
> I implemented COM Interop on top of the current remoting infrastructure.
> This allows for two main things. 1. Forwarding of method calls to the
> underlying unmanaged COM object. 2. Casting RCW (Runtime Callable Wrappers -
> the managed wrapper around the unmanaged COM object) to interfaces not
> specified as implemented in metadata. This can occur if a QueryInterface for
> that interface's Guid (IID) succeeds on the underlying object.
>
> So, when a user says "MyComObj obj = new MyComObj()" the runtime creates a
> ComInteropProxy and returns it's TransparentProxy. However, instead of
> forwarding methods via the remoting invoke mechanism using messages, I
> shortcut to a Com Interop invoke. The is a great performance boost (by a few
> orders of magnitude if I recall). I first emit the invoke call, which
> transitions the call from a call on the transparent proxy, into a call on
> the actuall RCW.
>
> The method implementation is done by cominterop_get_native_wrapper. This
> emits a method whose signature matches the managed method. The emitted
> method calls a final emitted method created by
> cominterop_get_native_wrapper_adjusted. The two methods,
> created by cominterop_get_native_wrapper and
> cominterop_get_native_wrapper_adjusted are 1-1. The
> adjusted method matches the unmanaged signature, and as thus can reuse all
> the existing unmanaged calling functionality provided by
> mono_marshal_emit_native_wrapper. The only small change to
> mono_marshal_emit_native_wrapper was that the function
> pointer is push onto the stack at call time, rather than when the method is
> emitted since the function pointer depends on the object making the call
> (different objects implementing the same interface could have different
> implementations, and thus vtables, thus function pointers).
>
> Another minor note is the internal calls added to __ComObject. This is so I
> can store a hashtable of COM interfaces and later release them in the
> finalizer to ensure proper reference counting. I had a Hashtable in managed
> code intially but couldn't access it in my finalizer.
>
> I also implemented a series of COM Interop related methods in the Marshal
> class.
>
> On windows you should be able to run some basic tests. I added a few to the
> cominterop.cs test file but disabled them for the moment. COM is a binary
> standard and I wasn't sure what vtable layouts would be like on Solaris,
> ARM, etc. (I know HP is different) and didn't want to cause regressions on
> those platforms. But, on windows/linux x86/64 you can enable them and run
> them. Or more excitingly you can add a reference to an interop assembly on
> MS and test run real COM objects (Internet Explorer for example). The
> largest thing missing right now is the marshalling of com objects. So, you
> can't call any methods that have parameters/return value of COM objects.
> This will be the next thing I work on.
>
> I'm sure there are some issues, so please review and I'll fix them ASAP.
>
> Thanks,
> Jonathan
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
>
>