[Mono-dev] [PATCH] Add mixed-mode assembly support on Windows (now build with cygwin as well)

[Kornél Pál]

Hi,

Attached the updated patch.

I realized that mono_marshal_get_vtfixup_ftnptr was broken and I 
reimplemented it without using mono_marshal_get_managed_wrapper.

I also have done some modifications to gc.c.

There is a comment in the patch /* FIXME: do we need this? (image is 
disposed anyway) */ about some code I think should be removed because it's 
unneeded. Is that code required?

Kornél

> Thank you very much for the review and your comments.
>
>> From: Zoltan Varga
>> To: Kornél Pál
>> Cc: mono-devel
>> Sent: Friday, April 25, 2008 4:45 PM
>> Subject: Re: [Mono-dev] [PATCH] Add mixed-mode assembly support on 
>> Windows (now build with cygwin as well)
>
>
>> - appdomain.h: This is a public header file, so the new functions
>>   should go somewhere
>>   else, like domain-internals.h
>> - domain.c: The code here could be put into a function in corree.c.
>> - Put mono_module_handle and coree_handle into some header file like
>>   coree.h, and
>>   there definitons into corree.c for example.
>
>> - corree.h: Please include <config.h>, since that contains the
>>   definition of PLATFORM_WIN32.
>
> OK.
>
>> - image.c: mono_cli_rva_image_map is part of our public api, so its 
>> signature
>>   cannot be changed, you can rename it to 
>> mono_cli_rva_image_map_internal, and
>>   change the signature of that.
>
> I had a look at "C:\Program Files\Mono\include" and it doesn't contain any 
> declaration. Is this public for sure? If it is I'll do what you suggested.
>
>> - The usage of 'raw_data' is a bit confusing: sometimes it is used as an 
>> address
>>   (like in mono_cli_rva_image_map), sometimes it is used as a handle 
>> (everywhere
>>   else). Which is correct ?
>
> HMODULE is the actual pointer to the base address (position of the DOS 
> header) of the module. Also note that there is no such thing as HINSTANCE 
> it is a heritage of the Win16 shared address space and HMODULE and 
> HINSTANCE are the same in Win32. In Win32s (Win32 API on Win16 that is not 
> really Win32 compatible) HMODULE is definitely not the base address but 
> someting else that may be a handle. So raw_data is actually the raw_data 
> but unlike the current implementation sections are mapped to the actual 
> virtual addresses. Even checks in map functions could be omitted but I 
> didn't remove them to ensure that the image is correct.
>
>> - cil-coff.h: Please don't declare constants in the middle of a a
>>   structure definition.
>
> OK. I actually tried to follow what seemed to me being the coding 
> convention of that file.
>
>> - gc.c: The comment about mono_thread_attach () might be incorrect, but 
>> the
>>   thread_started_event stuff is definitely needed to avoid races when
>>   the runtime is
>>   shut down before the finalizer thread can start. So I don't think we
>>   can remove this.
>
> As I see there can be no race because there only are "mono_thread_current 
> () == gc_thread" checks. Even if "mono_thread_current ()" returns NULL 
> (that is unlikely I think) nothing will happen on the thread if the 
> finalizer thread in not initialized yet. Actual work is only done by 
> "finalizer_thread" that is being executed only in the finalizer thread. If 
> for some reason "mono_gc_cleanup ()" is called before the finalizer thread 
> is initialized "WaitForSingleObjectEx" will wait for two seconds for the 
> finalizers and thus for the finalizer thread being initialized. If it 
> fails to initialize within two seconds finalizers won't run of course but 
> the same is true for finalizers that werent processed in two seconds so I 
> see no big difference. If really need "WaitForSingleObjectEx 
> (thread_started_event, INFINITE, FALSE);" can be moved just before 
> "WaitForSingleObjectEx (shutdown_event, 2000, FALSE)" but I don't see any 
> reason to do so. If I'm wrong please let me know.
>
> Kornél 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mixed_mode8.diff.txt
Url: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20080426/30a3ddbd/attachment-0001.txt