[Mono-dev] [PATCH] Add mixed-mode assembly support on Windows (now build with cygwin as well)
Zoltan Varga
vargaz at gmail.com
Fri Apr 25 10:45:52 EDT 2008
Hi,
Some comments to the patch:
- 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.
- corree.h: Please include <config.h>, since that contains the
definition of PLATFORM_WIN32.
- Put mono_module_handle and coree_handle into some header file like
coree.h, and
there definitons into corree.c for example.
- 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.
- 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 ?
- cil-coff.h: Please don't declare constants in the middle of a a
structure definition.
- 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.
Zoltan
2008/4/25 Kornél Pál <kornelpal at gmail.com>:
> Hi,
>
> Previously I forgot to try _CorDllMain to load the runtime when it hasn't
> been loaded yet. As a result I found that thread_started_event in
> mono/mono/metadata/gc.c causes a deadlock in this situation because
> _CorDllMain is callled while the OS loader lock is held by the current
> thread and no new threads are started while OS loader lock is held. After
> examining the purpose of thread_started_event I found that unlike the
> comment states it is not required at all so I removed it completely. This
> will make startup faster as well. I also checked that finalizer_thread is
> called and reaches SetEvent (shutdown_event) at the end.
>
> My test confirmed that Mono even supports mixed-mode assemblies with
> exported symbols if they were native DLLs just like MS.NET does.
>
>
> Kornél
>
> ----- Original Message ----- From: Kornél Pál
> To: mono-devel
> Sent: Friday, April 25, 2008 12:31 AM
>
>
> Subject: Re: [Mono-dev] [PATCH] Add mixed-mode assembly support on Windows
> (now build with cygwin as well)
>
>
>
> > Hi,
> >
> > I also modified Makefile to export __stdcall functions without mangling
> and
> > create-windef.pl to include MonoFixupCorEE. Other than these there are no
> > modifications.
> >
> > Kornél
> >
> > ----- Original Message ----- From: Kornél Pál
> > To: mono-devel; Robert Jordan
> > Sent: Wednesday, April 23, 2008 2:40 PM
> > Subject: Re: [Mono-dev] [PATCH] Add mixed-mode assembly support on Windows
> > (now build with cygwin as well)
> >
> >
> >
> > >
> > > > From: Robert Jordan
> > > > I think the following code is from another patch set (the cmd line
> > > > encoding issue you sent a patch for). Is it complete?
> > > > Index: mono/mono/mini/main.c
> > > >
> > >
> > > Yes, it is. I included it in this patch to be consistent with
> _CorExeMain
> > > implementation. Also note that command line arguments are parsed as
> UTF-8
> > > without MONO_EXTERNAL_ENCODINGS. MONO_EXTERNAL_ENCODINGS should not be
> > > supported on Windows, Unicode API should be used instead. Patching main
> > > will
> > > make Mono support non-ASCII command line arguments on Windows. Also note
> > > that because images are loaded using LoadLibrary file names are expected
> > > to
> > > be in UTF-8 that is supported by this main function patch. I still
> would
> > > vote for removing MONO_EXTERNAL_ENCODINGS support on Windows but that is
> > > out
> > > of the scope of this patch.
> > >
> > >
> > > > If this is solved, I will try to fix the cygwin build.
> > > >
> > >
> > > I managed to fix the build. #include <config.h> had to be moved out of
> > > #ifdef PLATFORM_WIN32 because PLATFORM_WIN32 is defined in config.h in
> the
> > > cygwin build.
> > >
> > > Please review the patch. Please try it on Linux as well to make sure
> that
> > > it
> > > doesn't break Linux. And if you like it, please approve the patch.
> > >
> > > Kornél
> > >
> > >
> >
> >
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>
More information about the Mono-devel-list
mailing list