[Mono-dev] [PATCH] Fixes and improvements for mixed-mode assembly support

[Kornél Pál]

Hi,

Thanks for the review.

threads_initialized is required by CorExitProcess. It is intended to shut 
down the runtime (including calling finalizers) properly and is called by 
msvcrt.dll (there are a lot of versions with different names) as well. I 
think that mono_thread_suspend_all_other_threads () is required for proper 
shutdown but CorExitProcess may be called at any time so 
mono_thread_suspend_all_other_threads () can fail with with an exception 
because the critical section was not initialized. I think that the only 
situation when threads_initialized is needed is when Mono itself calls 
"exit" (implemented in msvcrt.dll) while initializing that will call back to 
CorExitProcess on the same thread so there should not be a race.

If I'll implement CorBindToRuntimeEx I'll most likely add an initialization 
lock in coree.c that will eliminate any possible races in initialization and 
shutdown (including CorExitProcess) functions but I think the current code 
is good for now. Note that a proper CorBindToRuntimeEx implementation would 
need to eliminate calling "exit" on failures because the current 
initialization routines assume that they are are in mono.exe that has to be 
terminated on failure.

The renaming is not needed but I forgot to clean this up in the original 
check-in and makes naming convention inconsistent. I'll commit that as a 
separate revision.

Note that tests having Msvcrt in their names are not woring on Mono yet.

Kornél

----- Original Message ----- 
From: "Rodrigo Kumpera" <kumpera at gmail.com>
To: "Kornél Pál" <kornelpal at gmail.com>
Cc: <mono-devel-list at lists.ximian.com>
Sent: Friday, May 09, 2008 3:17 PM
Subject: Re: [Mono-dev] [PATCH] Fixes and improvements for mixed-mode 
assembly support


Hi Kornél,

What is the reason of your changes in threads.c? I can't see it making any
diference and it's possibly racy as there is no membar around the new static
variable.

@@ -203,7 +221,7 @@
  IMAGE_DOS_HEADER* DosHeader;
  IMAGE_NT_HEADERS* NtHeaders;
  DWORD* Address;
- DWORD dwOldProtect;
+ DWORD OldProtect;


Please avoid this variable renaming as it bloats the patch. If renaming is
really needed, it should be a separate patch.


Thanks,
Rodrigo



2008/5/9 Kornél Pál <kornelpal at gmail.com>:

> Hi,
>
> Mixed-mode assembly support is currently (temporarily) disabled in SVN.
> This patch is supposed to fix the issue reported by Jonathan and reenable
> mixed-mode assembly support.
>
> May I commit this patch?
>
> Also note that I have added some mixed-mode assembly tests in r102861 that
> can be compiled using Visual Studio 2005 or later.
> MixedModeLibrary/NativeApp is a special case because it tests _CorDllMain
> and loads mono.dll so has to be executed without mono.exe.
>
> Kornél
>
> ----- Original Message ----- From: "Kornél Pál"
> To: "mono-devel"
> Cc: "Zoltan Varga"; "Jonathan Chambers"
> Sent: Monday, May 05, 2008 10:56 AM
> Subject: [PATCH] Fixes and improvements for mixed-mode assembly support
>
>
>
>  Hi,
>>
>> * domain.c: Open exe_image anyway so that it can be fixed up. Add a
>> mono_close_exe_image function that closes exe_image.
>>
>> * image.c: Fix reference counting when DLL image is loaded using
>> LoadLibrary
>> outside of image.c.
>>
>> * threads.c: Add threads_initialized that enables coree.c to call
>> mono_thread_suspend_all_other_threads at any time.
>>
>> * coree.c: Some improvements and add mono_set_act_ctx that loads manifest
>> file from the main assembly when process is created from mono.exe. This 
>> is
>> required for MSVCRT support but may be used by other assemblies as well
>> for
>> example for XP Visual Style support.
>>
>> Please review the patch and if you like it, please approve it.
>>
>> Jonathan, please try this patch if it fixes the issue you have reported.
>>
>> Kornél
>>
>>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>