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

[Kornél Pál]

Hi,

Zoltán's patch fixes some other things unrelated to my problem. I have a 
problem with an exception thrown by mono_threads_lock (EnterCriticalSection) 
because the critical section not being initialized. For now I have decided 
not to support proper shutdown in CorExitProcess because in addition it 
causes waiting for 2 seconds twice instead of once so a better solution 
involving changes to mini would be required.

Attached the reduced patch.

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: Monday, May 12, 2008 3:30 PM
Subject: Re: [Mono-dev] [PATCH] Fixes and improvements for mixed-mode 
assembly support


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 embedded and charset-unspecified text was scrubbed...
Name: coreefix.diff.txt
Url: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20080512/cfec328a/attachment-0001.txt