---- Reported by grompf@sublimeintervention.com 2004-09-28 22:42:36 MST ----

Currently delegates are improperly marshalled when returning from a PInvoke call; the following 
patch addresses this problem.



---- Additional Comments From grompf@sublimeintervention.com 2004-09-28 22:43:07 MST ----

Created an attachment (id=166848)
patch




---- Additional Comments From grompf@sublimeintervention.com 2004-09-28 22:44:38 MST ----

Created an attachment (id=166849)
testcase




---- Additional Comments From miguel@ximian.com 2004-09-28 22:48:14 MST ----

Am CCing Zoltan for approval powers.



---- Additional Comments From grompf@sublimeintervention.com 2004-09-28 23:04:55 MST ----

Created an attachment (id=166850)
cleaned up patch




---- Additional Comments From grompf@sublimeintervention.com 2004-09-28 23:06:46 MST ----

Zoltan,

  This currently walks the finalizable objects hash; BenM has expressed that with a new gc 
this might not be publically exportable anymore.  It would be better for performance to 
probably register all marshalled delegates in a hashtable and register the hashtable entry 
with the gc_finalizer.  Input/thoughts?

-kangaroo




---- Additional Comments From grompf@sublimeintervention.com 2004-09-29 01:18:10 MST ----

Created an attachment (id=166851)
updated patch




---- Additional Comments From grompf@sublimeintervention.com 2004-09-29 01:20:31 MST ----

Zoltan,

  the 3rd patch doesn't search the finalizer ghash, but implements its own that adds a 
delegate ot the hash as it gets marshalled down, and removes it when the delegate gets 
finalized.

  Additionally it brings the exception for a delegate created in native land into line with 
MS.Net:

Unhandled Exception: System.ArgumentException: Additional information: Function 
pointer was not created by a delegate.
in (unmanaged) (wrapper managed-to-native) System.Object:
__icall_wrapper_mono_ftn_to_delegate (intptr,intptr)

  The only place this might not be perfect yet is the handling of the delegates in the hash; 
when marshalling the delegate_trampoline back to a delegate, should we clone the object; 
or keep things how we are by just pointing the MonoObject/MonoDelegate to the same 
object?

-kangaroo




---- Additional Comments From vargaz@gmail.com 2004-09-29 09:13:31 MST ----

Some comments:
- the functionality required is already implemented in 
  mono_ftnptr_to_delegate, so there is no need to add new functions/
fields to Delegate. Instead, mono_ftnptr_to_delegate should be modified
to include the code which is currently in mono_ftn_to_delegate.
- hash table access needs to be synchronized using marshal_mutex.




---- Additional Comments From grompf@sublimeintervention.com 2004-09-29 10:50:45 MST ----

Zoltan,

  mono_ftnptr_to_delegate actually does something different, it takes the addr of the 
MonoObject and marshals back to a delegate. This is currently being called on line 2687 
of marshal.c.  mono_ftn_to_delegate is what I added to deal with mapping a 
delegate_trampoline (what gets marshalled when a delegate is pinvoked) back to a 
MonoDelegate  Its a difference of method_addr vs delegate_trampoline.  The reason for 
the field added to Delegate was to ensure that anything added to the hashtable was 
removed when the gc was fired on an object, I dont see how anything in 
mono_ftnptr_to_delegate does things differently.

  Is line 2687 a mistaken entry and is what you're saying that mono_ftnptr_to_delegate is 
actually supposed to be doing what I have mono_ftn_to_delegate doing?

  I'll sync the ht to the mutex in the mean time;

-kangaroo
 



---- Additional Comments From grompf@sublimeintervention.com 2004-09-29 12:00:16 MST ----

Created an attachment (id=166852)
updated patch




---- Additional Comments From grompf@sublimeintervention.com 2004-09-29 12:02:00 MST ----

Zoltan,

  that latest patch moves everything into mono_ftnptr_to_delegate and drops 
mono_ftn_to_delegate.  I tested the code on line 2687 which appears to happen when a 
delegate in native land invokes with a delegate as a argument.  That works as well.  I also 
introduced the mutex locks that you wanted.

-kangaroo




---- Additional Comments From vargaz@freemail.hu 2004-09-29 12:23:54 MST ----

delegate_hash_table_remove (this) could be called from
mono_delegate_free_ftnptr () so there is no need
for the new icall and the delegate_marshalled field in Delegate.




---- Additional Comments From grompf@sublimeintervention.com 2004-09-29 12:49:14 MST ----

Created an attachment (id=166853)
updated patch




---- Additional Comments From vargaz@gmail.com 2004-09-29 13:13:17 MST ----

The marshal.h changes are not needed any more. Other than that, this is
ok to check in.



---- Additional Comments From grompf@sublimeintervention.com 2004-09-29 13:27:39 MST ----

final changes made and commited to HEAD

thanks zoltan

Imported an attachment (id=166848)
Imported an attachment (id=166849)
Imported an attachment (id=166850)
Imported an attachment (id=166851)
Imported an attachment (id=166852)
Imported an attachment (id=166853)

Unknown operating system unknown. Setting to default OS "Other".