Proper app domain separation regarding thread abort exception state.
authorMark Probst <mark.probst@gmail.com>
Fri, 17 Apr 2009 23:01:23 +0000 (23:01 -0000)
committerMark Probst <mark.probst@gmail.com>
Fri, 17 Apr 2009 23:01:23 +0000 (23:01 -0000)
2009-04-18  Mark Probst  <mark.probst@gmail.com>

        * icall-def.h: New icall for getting the thread abort exception
        state for a thread.

        * object.c, thread.c, object-internals.h: A thread's abort
        exception state is now a GC handle.  To get the object it stands
        for, we must move it into the current app domain, if it's
        different than the one where it originated from.

        * appdomain.c: Bump corlib version.

        * domain.c, domain-internals.h: New function for setting the
        domain and migrate the thread abort exception or not.

2009-04-18  Mark Probst  <mark.probst@gmail.com>

        * appdomain-thread-abort.cs: New tests for thread aborts across
        app domains.

        * Makefile.am: Test added.

svn path=/trunk/mono/; revision=132064

12 files changed:
mono/metadata/ChangeLog
mono/metadata/appdomain.c
mono/metadata/domain-internals.h
mono/metadata/domain.c
mono/metadata/icall-def.h
mono/metadata/object-internals.h
mono/metadata/object.c
mono/metadata/threads-types.h
mono/metadata/threads.c
mono/tests/ChangeLog
mono/tests/Makefile.am
mono/tests/appdomain-thread-abort.cs [new file with mode: 0644]

index 0618ce491ed412891af401b47efba8aff254c904..f7e3afaaccfa4d528491631cdd928f0efd6f8ba6 100644 (file)
@@ -1,3 +1,18 @@
+2009-04-18  Mark Probst  <mark.probst@gmail.com>
+
+       * icall-def.h: New icall for getting the thread abort exception
+       state for a thread.
+
+       * object.c, thread.c, object-internals.h: A thread's abort
+       exception state is now a GC handle.  To get the object it stands
+       for, we must move it into the current app domain, if it's
+       different than the one where it originated from.
+
+       * appdomain.c: Bump corlib version.
+
+       * domain.c, domain-internals.h: New function for setting the
+       domain and migrate the thread abort exception or not.
+
 2009-04-16 Rodrigo Kumpera  <rkumpera@novell.com>
 
        * metadata-verify.c: Add initial verification of the
index ea9d5ff7c3e5df057120d8392fb331771596ded7..dbf68ed2ebeaee654a59038d1f834dbde704e689 100644 (file)
@@ -71,7 +71,7 @@
  * Changes which are already detected at runtime, like the addition
  * of icalls, do not require an increment.
  */
-#define MONO_CORLIB_VERSION 74
+#define MONO_CORLIB_VERSION 75
 
 typedef struct
 {
index 3acb87161ce70de4a4e80f12f263b1af1fa6a4f2..9d06ac4c4d79697cc5183141b1ed349e90575088 100644 (file)
@@ -352,6 +352,9 @@ mono_domain_code_commit (MonoDomain *domain, void *data, int size, int newsize)
 void
 mono_domain_code_foreach (MonoDomain *domain, MonoCodeManagerFunc func, void *user_data) MONO_INTERNAL;
 
+void
+mono_domain_set_internal_with_options (MonoDomain *domain, gboolean migrate_exception) MONO_INTERNAL;
+
 /* 
  * Installs a new function which is used to return a MonoJitInfo for a method inside
  * an AOT module.
index 5e89522e6bfc8fe5d65a22d3be7a35c464ae16e2..27b9e680774288caca487063ec5e9b42b3441b73 100644 (file)
@@ -1754,6 +1754,25 @@ mono_domain_get ()
        return GET_APPDOMAIN ();
 }
 
+void
+mono_domain_set_internal_with_options (MonoDomain *domain, gboolean migrate_exception)
+{
+       MonoThread *thread;
+
+       SET_APPDOMAIN (domain);
+       SET_APPCONTEXT (domain->default_context);
+
+       if (migrate_exception) {
+               thread = mono_thread_current ();
+               if (!thread->abort_exc)
+                       return;
+
+               g_assert (thread->abort_exc->object.vtable->domain != domain);
+               MONO_OBJECT_SETREF (thread, abort_exc, mono_get_exception_thread_abort ());
+               g_assert (thread->abort_exc->object.vtable->domain == domain);
+       }
+}
+
 /**
  * mono_domain_set_internal:
  * @domain: the new domain
@@ -1763,8 +1782,7 @@ mono_domain_get ()
 void
 mono_domain_set_internal (MonoDomain *domain)
 {
-       SET_APPDOMAIN (domain);
-       SET_APPCONTEXT (domain->default_context);
+       mono_domain_set_internal_with_options (domain, TRUE);
 }
 
 void
index da9198b07c99e1c725913c1b53dfd5e859960654..78eae543a6f5784b859f7f4cc89bb657be405772 100644 (file)
@@ -858,6 +858,7 @@ ICALL(THREAD_1, "Abort_internal(object)", ves_icall_System_Threading_Thread_Abor
 ICALL(THREAD_2, "ClrState", ves_icall_System_Threading_Thread_ClrState)
 ICALL(THREAD_3, "CurrentThread_internal", mono_thread_current)
 ICALL(THREAD_4, "FreeLocalSlotValues", mono_thread_free_local_slot_values)
+ICALL(THREAD_55, "GetAbortExceptionState", ves_icall_System_Threading_Thread_GetAbortExceptionState)
 ICALL(THREAD_5, "GetCachedCurrentCulture", ves_icall_System_Threading_Thread_GetCachedCurrentCulture)
 ICALL(THREAD_6, "GetCachedCurrentUICulture", ves_icall_System_Threading_Thread_GetCachedCurrentUICulture)
 ICALL(THREAD_7, "GetDomainID", ves_icall_System_Threading_Thread_GetDomainID)
index c0120defa0cd3593b8fe62b6af6b97d48e47965b..0332b81c8ccf456bf23c28a3e593114dde398a31 100644 (file)
@@ -320,7 +320,7 @@ struct _MonoThread {
        guint32     name_len;
        guint32     state;
        MonoException *abort_exc;
-       MonoObject *abort_state;
+       int abort_state_handle;
        guint64 tid;    /* This is accessed as a gsize in the code (so it can hold a 64bit pointer on systems that need it), but needs to reserve 64 bits of space on all machines as it corresponds to a field in managed code */
        HANDLE      start_notify;
        gpointer stack_ptr;
index f33ccab7daf6d471b25e45e394fdcd69d56cac7c..4d130bbe77e4e6f5acc34e05695906fefff6ee30 100644 (file)
@@ -4942,8 +4942,14 @@ mono_raise_exception (MonoException *ex)
         * will point into the next function in the executable, not this one.
         */
 
-       if (((MonoObject*)ex)->vtable->klass == mono_defaults.threadabortexception_class)
-               MONO_OBJECT_SETREF (mono_thread_current (), abort_exc, ex);
+       if (((MonoObject*)ex)->vtable->klass == mono_defaults.threadabortexception_class) {
+               MonoThread *thread = mono_thread_current ();
+               g_assert (ex->object.vtable->domain == mono_domain_get ());
+               MONO_OBJECT_SETREF (thread, abort_exc, ex);
+               if (thread->abort_state_handle)
+                       mono_gchandle_free (thread->abort_state_handle);
+               thread->abort_state_handle = 0;
+       }
        
        ex_handler (ex);
 }
index 4cb033b7994d58a17af1b4ef809cbe2e15c81df5..e15b1fcf8719318408be97cc42bae7781167cd46 100644 (file)
@@ -119,6 +119,7 @@ gint64 ves_icall_System_Threading_Interlocked_Decrement_Long(gint64 * location)
 
 void ves_icall_System_Threading_Thread_Abort (MonoThread *thread, MonoObject *state) MONO_INTERNAL;
 void ves_icall_System_Threading_Thread_ResetAbort (void) MONO_INTERNAL;
+MonoObject* ves_icall_System_Threading_Thread_GetAbortExceptionState (MonoThread *thread) MONO_INTERNAL;
 void ves_icall_System_Threading_Thread_Suspend (MonoThread *thread) MONO_INTERNAL;
 void ves_icall_System_Threading_Thread_Resume (MonoThread *thread) MONO_INTERNAL;
 void ves_icall_System_Threading_Thread_ClrState (MonoThread *thread, guint32 state) MONO_INTERNAL;
index a7d76d65ddd53ff7da181eec75c479d4fd72fa15..13da2d488f50804cf39cad51e5c48f61fb577e75 100644 (file)
@@ -1033,6 +1033,12 @@ void ves_icall_System_Threading_Thread_Thread_free_internal (MonoThread *this,
        DeleteCriticalSection (this->synch_cs);
        g_free (this->synch_cs);
        this->synch_cs = NULL;
+
+       if (this->abort_state_handle) {
+               g_assert (this->abort_exc);
+               mono_gchandle_free (this->abort_state_handle);
+               this->abort_state_handle = 0;
+       }
 }
 
 static void mono_thread_start (MonoThread *thread)
@@ -2121,7 +2127,14 @@ ves_icall_System_Threading_Thread_Abort (MonoThread *thread, MonoObject *state)
        }
 
        thread->state |= ThreadState_AbortRequested;
-       MONO_OBJECT_SETREF (thread, abort_state, state);
+       if (thread->abort_state_handle)
+               mono_gchandle_free (thread->abort_state_handle);
+       if (state) {
+               thread->abort_state_handle = mono_gchandle_new (state, FALSE);
+               g_assert (thread->abort_state_handle);
+       } else {
+               thread->abort_state_handle = 0;
+       }
        thread->abort_exc = NULL;
 
        LeaveCriticalSection (thread->synch_cs);
@@ -2155,12 +2168,138 @@ ves_icall_System_Threading_Thread_ResetAbort (void)
                mono_raise_exception (mono_get_exception_thread_state (msg));
        } else {
                thread->abort_exc = NULL;
-               thread->abort_state = NULL;
+               if (thread->abort_state_handle) {
+                       mono_gchandle_free (thread->abort_state_handle);
+                       /* This is actually not necessary - the handle
+                          only counts if the exception is set */
+                       thread->abort_state_handle = 0;
+               }
        }
        
        LeaveCriticalSection (thread->synch_cs);
 }
 
+static MonoObject*
+serialize_object (MonoObject *obj, gboolean *failure, MonoObject **exc)
+{
+       static MonoMethod *serialize_method;
+
+       void *params [1];
+       MonoObject *array;
+
+       if (!serialize_method) {
+               MonoClass *klass = mono_class_from_name (mono_defaults.corlib, "System.Runtime.Remoting", "RemotingServices");
+               serialize_method = mono_class_get_method_from_name (klass, "SerializeCallData", -1);
+       }
+
+       if (!serialize_method) {
+               *failure = TRUE;
+               return NULL;
+       }
+
+       g_assert (!obj->vtable->klass->marshalbyref);
+
+       params [0] = obj;
+       *exc = NULL;
+       array = mono_runtime_invoke (serialize_method, NULL, params, exc);
+       if (*exc)
+               *failure = TRUE;
+
+       return array;
+}
+
+static MonoObject*
+deserialize_object (MonoObject *obj, gboolean *failure, MonoObject **exc)
+{
+       static MonoMethod *deserialize_method;
+
+       void *params [1];
+       MonoObject *result;
+
+       if (!deserialize_method) {
+               MonoClass *klass = mono_class_from_name (mono_defaults.corlib, "System.Runtime.Remoting", "RemotingServices");
+               deserialize_method = mono_class_get_method_from_name (klass, "DeserializeCallData", -1);
+       }
+       if (!deserialize_method) {
+               *failure = TRUE;
+               return NULL;
+       }
+
+       params [0] = obj;
+       *exc = NULL;
+       result = mono_runtime_invoke (deserialize_method, NULL, params, exc);
+       if (*exc)
+               *failure = TRUE;
+
+       return result;
+}
+
+static MonoObject*
+make_transparent_proxy (MonoObject *obj, gboolean *failure, MonoObject **exc)
+{
+       static MonoMethod *get_proxy_method;
+
+       MonoDomain *domain = mono_domain_get ();
+       MonoRealProxy *real_proxy;
+       MonoReflectionType *reflection_type;
+       MonoTransparentProxy *transparent_proxy;
+
+       if (!get_proxy_method)
+               get_proxy_method = mono_class_get_method_from_name (mono_defaults.real_proxy_class, "GetTransparentProxy", 0);
+
+       g_assert (obj->vtable->klass->marshalbyref);
+
+       real_proxy = (MonoRealProxy*) mono_object_new (domain, mono_defaults.real_proxy_class);
+       reflection_type = mono_type_get_object (domain, &obj->vtable->klass->byval_arg);
+
+       real_proxy->class_to_proxy = reflection_type;
+       real_proxy->unwrapped_server = obj;
+
+       *exc = NULL;
+       transparent_proxy = (MonoTransparentProxy*) mono_runtime_invoke (get_proxy_method, real_proxy, NULL, exc);
+       if (*exc)
+               *failure = TRUE;
+
+       return (MonoObject*) transparent_proxy;
+}
+
+MonoObject*
+ves_icall_System_Threading_Thread_GetAbortExceptionState (MonoThread *thread)
+{
+       MonoObject *state, *serialized, *deserialized, *exc;
+       MonoDomain *domain;
+       gboolean failure = FALSE;
+
+       if (!thread->abort_state_handle)
+               return NULL;
+
+       state = mono_gchandle_get_target (thread->abort_state_handle);
+       g_assert (state);
+
+       domain = mono_domain_get ();
+       if (state->vtable->domain == domain)
+               return state;
+
+       if (state->vtable->klass->marshalbyref) {
+               deserialized = make_transparent_proxy (state, &failure, &exc);
+       } else {
+               mono_domain_set_internal_with_options (state->vtable->domain, FALSE);
+               serialized = serialize_object (state, &failure, &exc);
+               mono_domain_set_internal_with_options (domain, FALSE);
+               if (!failure)
+                       deserialized = deserialize_object (serialized, &failure, &exc);
+       }
+
+       if (failure) {
+               MonoException *invalid_op_exc = mono_get_exception_invalid_operation ("Thread.ExceptionState cannot access an ExceptionState from a different AppDomain");
+               if (exc)
+                       MONO_OBJECT_SETREF (invalid_op_exc, inner_ex, exc);
+               mono_raise_exception (invalid_op_exc);
+       }
+
+       return deserialized;
+}
+
 static gboolean
 mono_thread_suspend (MonoThread *thread)
 {
index d02913b39b0abc7f7549eb1da9227aa061c64352..4ece1ee34375448ebda5d342efab07fcd454fc62 100644 (file)
@@ -1,3 +1,10 @@
+2009-04-18  Mark Probst  <mark.probst@gmail.com>
+
+       * appdomain-thread-abort.cs: New tests for thread aborts across
+       app domains.
+
+       * Makefile.am: Test added.
+
 2009-04-17  Sebastien Pouliot  <sebastien@ximian.com>
 
        * coreclr-security.cs: Add test case for the "special" case about
index 677740d6f4f831a12ebc50f83ca7d73862d5e6c6..70baff5d5c5181496b65e732d69255d8be78e265 100644 (file)
@@ -352,7 +352,8 @@ BASE_TEST_CS_SRC=           \
        interlocked-3.cs        \
        interlocked-4.2.cs      \
        finalizer-wait.cs       \
-       critical-finalizers.cs
+       critical-finalizers.cs  \
+       appdomain-thread-abort.cs
 
 if AMD64
 TEST_CS_SRC = $(BASE_TEST_CS_SRC) async-exc-compilation.cs
diff --git a/mono/tests/appdomain-thread-abort.cs b/mono/tests/appdomain-thread-abort.cs
new file mode 100644 (file)
index 0000000..c4fa9ea
--- /dev/null
@@ -0,0 +1,175 @@
+using System;
+using System.Threading;
+using System.Runtime.Remoting;
+
+public class JustSomeClass {
+}
+
+public class Test : MarshalByRefObject {
+    ThreadAbortException exc;
+    public JustSomeClass other;
+
+    public void doThrow (int n, object state) {
+       if (n <= 0)
+           Thread.CurrentThread.Abort (state);
+       else
+           doThrow (n - 1, state);
+    }
+
+    public void abortProxy () {
+       doThrow (10, this);
+    }
+
+    public void abortOther () {
+       other = new JustSomeClass ();
+       doThrow (10, other);
+    }
+
+    public void abortString () {
+       try {
+           doThrow (10, "bla");
+       } catch (ThreadAbortException e) {
+           exc = e;
+       }
+    }
+
+    public void abortOtherIndirect (Test test) {
+       test.abortOther ();
+    }
+
+    public object getState () {
+       return exc.ExceptionState;
+    }
+
+    public int getInt () {
+           return 123;
+    }
+}
+
+public class main {
+    public static int Main (string [] args) {
+       AppDomain domain = AppDomain.CreateDomain ("newdomain");
+       Test test = (Test) domain.CreateInstanceAndUnwrap (typeof (Test).Assembly.FullName, typeof (Test).FullName);
+       bool didAbort;
+       Test testHere = new Test ();
+
+       if (!RemotingServices.IsTransparentProxy (test)) {
+           Console.WriteLine ("test is no proxy");
+           return 5;
+       }
+
+       try {
+           test.abortOtherIndirect (testHere);
+       } catch (ThreadAbortException e) {
+           object state = e.ExceptionState;
+           Thread.ResetAbort ();
+           if ((JustSomeClass)state != testHere.other) {
+               Console.WriteLine ("other class not preserved in state");
+               return 16;
+           }
+       }
+
+       try {
+           didAbort = false;
+           test.abortString ();
+       } catch (ThreadAbortException e) {
+           object state;
+           state = e.ExceptionState;
+           Thread.ResetAbort ();
+           didAbort = true;
+           if (state == null) {
+                   Console.WriteLine ("state is null");
+                   return 13;
+           } else {
+                   if (RemotingServices.IsTransparentProxy (state)) {
+                           Console.WriteLine ("state is proxy");
+                           return 1;
+                   }
+                   if (!((string)state).Equals ("bla")) {
+                           Console.WriteLine ("state is wrong: " + (string)state);
+                           return 2;
+                   }
+           }
+           if (RemotingServices.IsTransparentProxy (e)) {
+               Console.WriteLine ("exception is proxy");
+               return 3;
+           }
+           if (test.getState () != null) {
+               Console.WriteLine ("have state");
+               return 12;
+           }
+       }
+       if (!didAbort) {
+           Console.WriteLine ("no abort");
+           return 4;
+       }
+
+       try {
+           didAbort = false;
+           test.abortProxy ();
+       } catch (ThreadAbortException e) {
+           object state;
+           state = e.ExceptionState;
+           Thread.ResetAbort ();
+           didAbort = true;
+           if (state == null) {
+                   Console.WriteLine ("state is null");
+                   return 14;
+           } else {
+                   if (!RemotingServices.IsTransparentProxy (state)) {
+                           Console.WriteLine ("state is not proxy");
+                           return 6;
+                   }
+                   if (((Test)state).getInt () != 123) {
+                           Console.WriteLine ("state doesn't work");
+                           return 15;
+                   }
+           }
+           if (RemotingServices.IsTransparentProxy (e)) {
+                   Console.WriteLine ("exception is proxy");
+                   return 7;
+           }
+       }
+       if (!didAbort) {
+           Console.WriteLine ("no abort");
+           return 8;
+       }
+
+       try {
+           didAbort = false;
+           test.abortOther ();
+       } catch (ThreadAbortException e) {
+           object state = null;
+           bool stateExc = false;
+
+           didAbort = true;
+
+           try {
+               state = e.ExceptionState;
+               Console.WriteLine ("have state");
+           } catch (Exception) {
+               stateExc = true;
+               /* FIXME: if we put this after the try/catch, mono
+                  quietly quits */
+               Thread.ResetAbort ();
+           }
+           if (!stateExc) {
+               Console.WriteLine ("no state exception");
+               return 9;
+           }
+
+           if (RemotingServices.IsTransparentProxy (e)) {
+               Console.WriteLine ("exception is proxy");
+               return 10;
+           }
+       }
+       if (!didAbort) {
+           Console.WriteLine ("no abort");
+           return 11;
+       }
+
+       Console.WriteLine ("done");
+
+       return 0;
+    }
+}