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

Rodrigo Kumpera kumpera at gmail.com
Mon May 12 09:30:12 EDT 2008


Hi Kornél,

I think your changes to thread.c are no longer needed with the latest commit
from Zoltan. Mind testing your patch over his changes.

Thanks,
Rodrigo

On Fri, May 9, 2008 at 11:31 AM, Kornél Pál <kornelpal at gmail.com> wrote:

> 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
> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20080512/86875572/attachment.html 


More information about the Mono-devel-list mailing list