Re: [1/3] mscoree: Store the AppDomain from CorExeMain, for use in other functions.

[Juan Lang]

Hi Vincent,

+        EnterCriticalSection(&mono_lib_cs);
+        if (!mono_handle)
+        {
+            ERR("CorExitProcess called on another thread?\n");
+            return -1;
you don't release mono_lib_cs here.

Similarly, in patch 3,
 STDAPI ClrCreateManagedInstance(LPCWSTR pTypeName, REFIID riid, void
**ppObject)
(snip)
+        EnterCriticalSection(&mono_lib_cs);
+        if (!mono_handle)
+        {
+            ERR("CorExitProcess called on another thread?\n");
+            return E_FAIL;

you don't release mono_lib_cs here, and the error message looks suspicious.
--Juan

[1/3] mscoree: Store the AppDomain from CorExeMain, for use in other functions.

[Vincent Povirk]

> Hi Vincent,
>
> +        EnterCriticalSection(&mono_lib_cs);
> +        if (!mono_handle)
> +        {
> +            ERR("CorExitProcess called on another thread?\n");
> +            return -1;
> you don't release mono_lib_cs here.

Doh, thanks.
> Similarly, in patch 3,
>  STDAPI ClrCreateManagedInstance(LPCWSTR pTypeName, REFIID riid, void
> **ppObject)
> (snip)
> +        EnterCriticalSection(&mono_lib_cs);
> +        if (!mono_handle)
> +        {
> +            ERR("CorExitProcess called on another thread?\n");
> +            return E_FAIL;
>
> you don't release mono_lib_cs here, and the error message looks suspicious.
> --Juan

Yeah, I'm not really sure how this is supposed to work. load_mono()
ensures that mono is loaded, so mono_handle should be set if it
succeeds. Unless someone calls CorExitProcess in another thread and
unloads it before we can grab the critical section again.

Maybe load_mono should return without releasing the critical section,
thus preventing that race? The caller will almost always want to
proceed to create an AppDomain afterwards..

[1/3] mscoree: Store the AppDomain from CorExeMain, for use in other functions.

[Juan Lang]

> Yeah, I'm not really sure how this is supposed to work. load_mono()
> ensures that mono is loaded, so mono_handle should be set if it
> succeeds. Unless someone calls CorExitProcess in another thread and
> unloads it before we can grab the critical section again.
>
> Maybe load_mono should return without releasing the critical section,
> thus preventing that race? The caller will almost always want to
> proceed to create an AppDomain afterwards..

Ah, I see, you really did mean that.  No, I don't think leaving the
critical section open is the best thing.  I expect someone calling
CorExitProcess should expect problems if they continue to call mscoree
functions after that.
--Juan