[Mono-dev] COM Interop Patch

[Jon Chambers]

Thanks for the review Zoltan. I am addressing the issues you brought up. The
only issue is with the two methods needed for a COM call and their caching.
The first method (the one that calls the other generated method) is the one
I need to cache. You said I shouldn't mark it as wrapper type
MONO_WRAPPER_MANAGED_TO_NATIVE, which makes sense. I'll mark it
MONO_WRAPPER_COMINTEROP. The second generated method I'll mark
MONO_WRAPPER_MANAGED_TO_NATIVE, since it is. The second method never needs
looked up, so I will remove that cache. However, what cache should I put the
first method into; the native_wrapper_cache or a specific cache for
cominterop (I can use the cache I was using for the second emitted method,
since those methods don't need it).

Thanks,
Jonathan

On 7/14/06, Zoltan Varga <vargaz at gmail.com> wrote:
>
>                                          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
> >
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20060714/aa38a80c/attachment.html