+2006-05-17 Martin Baulig <martin@ximian.com>
+
+ Fix a weird race condition which prevented XSP from working inside
+ the debugger - see doc/debugger-issues.txt for details.
+
+ * include/gc.h: Moved the "libgc-mono-debugger.h" #include down
+ after the gc_pthread_redirects.h one.
+
+ * include/libgc-mono-debugger.h
+ (GCThreadFunctions): Added `thread_created' and `thread_exited'.
+ (GC_mono_debugger_add_all_threads): New function prototype.
+
+ * pthread_stop_world.c (gc_thread_vtable): Allow the vtable and
+ any function in it be NULL; use NULL as the default vtable.
+ (GC_mono_debugger_add_all_threads): New public function.
+
+ * pthread_support.c (GC_new_thread): Use calloc() instead of
+ GC_INTERNAL_MALLOC() to allocate the `GC_thread' structure.
+ (GC_delete_thread): Call `gc_thread_vtable->thread_exited()'.
+ (GC_thr_init): Call `gc_thread_vtable->thread_created()'.
+ (GC_start_routine_head): Likewise; use calloc() instead of
+ GC_INTERNAL_MALLOC() to allocate the `start_info'.
+
2006-04-05 Zoltan Varga <vargaz@gmail.com>
* include/private/gcconfig.h (LINUX and SPARC): Applied patch from
--- /dev/null
+I spent the last couple of days debugging a very weird race condition.
+
+The problem only occured when running XSP (SVN revision 60518) inside
+the debugger and only when using special parameters.
+
+I'm using Mono from SVN revision 60564 (that's from last Thursday),
+XSP from SVN revision 60518 and manually installed xsp.exe.mdb and
+Mono.WebServer.dll.mdb into $prefix/lib/xsp/1.0/.
+
+With this setup, I'm running XSP with
+
+ mdb -args /work/asgard/INSTALL/lib/xsp/1.0/xsp.exe --root /work/asgard/INSTALL/lib/xsp/test/
+
+Note that adding options like --nonstop or changing the --root may
+make the problem go away or make it crash somewhere else.
+
+Then I insert a breakpoint on line 476 (that's the line before the
+Console.ReadLine()) and continue.
+
+Using `set env GC_DONT_GC 1' inside mdb makes the problem go away and
+running a stand-alone mono with -O=shared (and all the other
+optimization flags the debugger is using) works fine.
+
+So my first guess was that this is a GC issue.
+
+After implementing hardware breakpoints in the debugger, I was finally
+able to track this down. If I understand things correctly, the
+problem goes like this:
+
+Some code inside XSP calls mono_thread_pool_add() - inside that
+method, we GC-allocate an `ASyncCall *ac' structure, store the `msg'
+and `state' objects in it and create a `MonoAsyncResult *ares'.
+
+Then we call mono_thread_create() passing it async_invoke_thread() and
+the `ares'.
+
+mono_thread_create() stores them as `func' and `start_arg' in the
+g_new()-allocated `start_info' and calls CreateThread() which calls
+pthread_create().
+
+pthread_create() is in fact a wrapper in libgc - it calls the "real"
+pthread_create() and then blocks on a semaphore until the thread is
+actually started.
+
+Now - somehow - and I still don't fully understand why - the parent
+"loses" all references to the `ac' and `ares' after calling the real
+pthread_create().
+
+If I understand this correctly, mono_thread_pool_add() only stores
+them in registers and not on the stack, so the `start_info' contains
+the only references to them. The `start_info', however, is just
+passed to the clone() system call and not accessed anymore after that.
+
+This means that all references to the `ac' and the `ares' may
+disappear from the parent's stack between the clone() and sem_wait()
+system calls. Under normal circumstances, this is no problem since
+the child's stack is created with a reference to the `start_info'.
+
+I said under normal circumstances, because this is where race
+condition #1 comes into the picture:
+
+The GC's pthread_create() passes a wrapper called GC_start_func()
+around the original `start_func' to the real pthread_create(). This
+wrapper calls GC_new_thread() and stores some information about the
+newly created thread in its internal structures - this information is
+also used to determine the child's stack.
+
+After that, it posts the semaphore on which the parent thread is
+blocking, we release the allocation lock and everything is fine.
+
+However - GC_new_thread() uses GC_INTERNAL_MALLOC() to allocate the
+`GC_thread' structure - and GC_INTERNAL_MALLOC() may in fact trigger a
+collection !
+
+Doing a collection at this time means we don't know about the child's
+stack yet - and if the parent doesn't keep a reference to the `ares'
+anymore, it's gone ....
+
+Fixing this was really easy, all I had to do is make GC_new_thread()
+use calloc() instead of GC_INTERNAL_MALLOC().
+
+The second issue is a debugger-only problem: we need to tell the
+debugger about newly created threads while still holding the
+allocation lock to ensure that no collection may happen in the
+meantime.
GC_API void (*GC_is_visible_print_proc)
GC_PROTO((GC_PTR p));
-#define _IN_LIBGC_GC_H
-#include "libgc-mono-debugger.h"
-
/* For pthread support, we generally need to intercept a number of */
/* thread library calls. We do that here by macro defining them. */
#endif
#endif
+#define _IN_LIBGC_GC_H
+#include "libgc-mono-debugger.h"
+
# if defined(PCR) || defined(GC_SOLARIS_THREADS) || \
defined(GC_PTHREADS) || defined(GC_WIN32_THREADS)
/* Any flavor of threads except SRC_M3. */
{
void (* initialize) (void);
+ void (* thread_created) (pthread_t tid, void *stack_ptr);
+ void (* thread_exited) (pthread_t tid, void *stack_ptr);
+
void (* stop_world) (void);
- void (* push_all_stacks) (void);
void (* start_world) (void);
} GCThreadFunctions;
extern GCThreadFunctions *gc_thread_vtable;
+extern void
+GC_mono_debugger_add_all_threads (void);
+
#else
#error "This header is only intended to be used by the Mono Debugger"
#endif
/* world is stopped. Should not fail if it isn't. */
void GC_push_all_stacks()
{
- gc_thread_vtable->push_all_stacks();
+ pthread_push_all_stacks();
}
/* There seems to be a very rare thread stopping problem. To help us */
#if DEBUG_THREADS
GC_printf1("Stopping the world from 0x%lx\n", pthread_self());
#endif
-
+
n_live_threads = GC_suspend_all();
if (GC_retry_signals) {
/* We should have previously waited for it to become zero. */
# endif /* PARALLEL_MARK */
++GC_stop_count;
- gc_thread_vtable->stop_world ();
+ if (gc_thread_vtable && gc_thread_vtable->stop_world)
+ gc_thread_vtable->stop_world ();
+ else
+ pthread_stop_world ();
# ifdef PARALLEL_MARK
GC_release_mark_lock();
# endif
void GC_start_world()
{
- gc_thread_vtable->start_world();
+ if (gc_thread_vtable && gc_thread_vtable->start_world)
+ gc_thread_vtable->start_world();
+ else
+ pthread_start_world ();
}
static void pthread_stop_init() {
/* We hold the allocation lock. */
void GC_stop_init()
{
- gc_thread_vtable->initialize ();
+ if (gc_thread_vtable && gc_thread_vtable->initialize)
+ gc_thread_vtable->initialize ();
+ else
+ pthread_stop_init ();
+}
+
+GCThreadFunctions *gc_thread_vtable = NULL;
+
+void
+GC_mono_debugger_add_all_threads (void)
+{
+ GC_thread p;
+ int i;
+
+ if (gc_thread_vtable && gc_thread_vtable->thread_created) {
+ for (i = 0; i < THREAD_TABLE_SZ; i++) {
+ for (p = GC_threads[i]; p != 0; p = p -> next) {
+ gc_thread_vtable->thread_created (p->id, &p->stop_info.stack_ptr);
+ }
+ }
+ }
}
-/*
- * This is used by the Mono Debugger to stop/start the world.
- */
-GCThreadFunctions pthread_thread_vtable = {
- pthread_stop_init,
-
- pthread_stop_world,
- pthread_push_all_stacks,
- pthread_start_world
-};
-
-GCThreadFunctions *gc_thread_vtable = &pthread_thread_vtable;
#endif
result = &first_thread;
first_thread_used = TRUE;
} else {
- result = (struct GC_Thread_Rep *)
- GC_INTERNAL_MALLOC(sizeof(struct GC_Thread_Rep), NORMAL);
+ result = calloc (1, sizeof (struct GC_Thread_Rep));
}
if (result == 0) return(0);
result -> id = id;
} else {
prev -> next = p -> next;
}
- GC_INTERNAL_FREE(p);
+ if (gc_thread_vtable && gc_thread_vtable->thread_exited)
+ gc_thread_vtable->thread_exited (id, &p->stop_info.stack_ptr);
+ free(p);
}
/* If a thread has been joined, but we have not yet */
} else {
prev -> next = p -> next;
}
- GC_INTERNAL_FREE(p);
+ free(p);
}
/* Return a GC_thread corresponding to a given pthread_t. */
GC_destroy_thread_local(p);
}
# endif /* THREAD_LOCAL_ALLOC */
- if (p != &first_thread) GC_INTERNAL_FREE(p);
+ if (p != &first_thread) free(p);
}
}
GC_threads[hv] = me;
t -> stop_info.stack_ptr = (ptr_t)(&dummy);
# endif
t -> flags = DETACHED | MAIN_THREAD;
+ if (gc_thread_vtable && gc_thread_vtable->thread_created)
+ gc_thread_vtable->thread_created (pthread_self (), &t->stop_info.stack_ptr);
GC_stop_init();
/* This is also < 100% convincing. We should also read this */
/* from /proc, but the hook to do so isn't there yet. */
# endif /* IA64 */
+ if (gc_thread_vtable && gc_thread_vtable->thread_created)
+ gc_thread_vtable->thread_created (my_pthread, &me->stop_info.stack_ptr);
UNLOCK();
if (start) *start = si -> start_routine;
/* responsibility. */
LOCK();
- si = (struct start_info *)GC_INTERNAL_MALLOC(sizeof(struct start_info),
- NORMAL);
+ si = calloc (1, sizeof (struct start_info));
UNLOCK();
if (!parallel_initialized) GC_init_parallel();
if (0 == si) return(ENOMEM);
}
sem_destroy(&(si -> registered));
LOCK();
- GC_INTERNAL_FREE(si);
+ free(si);
UNLOCK();
return(result);
projects / mono.git / commitdiff