[bug #24867] `define' should be thread-safe
Follow-up Comment #8, bug #24867 (project guile): Rochville University http://www.study-online.net/schools/rochville-university/ ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #9, bug #24867 (project guile): Thanks a lot for sharing such an valuable stuff. WEVAC University http://www.facebook.com/WEVACUniversity ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #10, bug #24867 (project guile): Almeda University http://answers.yahoo.com/question/index?qid=20110416135725AAw1nCL ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #11, bug #24867 (project guile): Belford university http://educationfuturist.wordpress.com/ Accredited high school diploma http://onlinediplomahelp.com/?p=38 ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #4, bug #24867 (project guile): Thanks for the patch and test case! A few remarks: 1. `scm_sym2var ()' must be changed to acquire the mutex before performing actual lookup. Thus, `scm_define ()' itself doesn't need to acquire it. I think this should be enough to fix the bug. It shouldn't slow things down too much, thanks to memoization. 2. The mutex can be declared as `static'. 3. In 1.9, we should perhaps avoid `scm_i_define_mutex' and use a per-module mutex (which isn't possible in 1.8 since it would break ABI). OTOH, is it really necessary to have such a fine grain? Can you provide an updated patch with ChangeLog-style entry? Ludo'. ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #5, bug #24867 (project guile): 1) Easier said than done, because: 1a) the mutex needs to be recursive, since sym2var evaluates code in boot9.scm which can cause sym2var to run again. The core problem is that the mechanism for specifying recursive mutexes seems to be somewhat OS-dependent (and possibly some OS'es don't support recursive mutexes??) and so a portability wrapper might be needed. :-( 1b) There's still a strange deadlock somehow; am debugging. 3) Fine-grained usually means speedy. *if* there was some per-module C struct, then the mutex could be put in there. (I don't know of any, but I don't know guile internals). The alternative would be somehow grabbing a lock in the boot9.scm code, but I don't see how, without making some symbol lookup (i.e. race). ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #6, bug #24867 (project guile): I've attached a patch to lock the entry and exit to scm_sym2var(). However, it doesn't fix the problem (at all). -- There are still deadlocks in garbage collection, (but different from the previously reported one !?) -- There are still crashes (with same stack trace as previously posted) -- There are still races. (file #17129) ___ Additional Item Attachment: File name: symvar-race.patch Size:1 KB ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #2, bug #24867 (project guile):
The following patch protects the update of the module hash tables
to be thread-safe. This is a partial solution to the bug reported
in https://savannah.gnu.org/bugs/?24867 This is not a full solution,
because other threads might still be reading the hash tables while
they are being updated, and thus may obtain stale/bad data.
Signed-off-by: Linas Vepstas linasveps...@gmail.com
---
libguile/modules.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
Index: guile-1.8.6/libguile/modules.c
===
--- guile-1.8.6.orig/libguile/modules.c 2008-12-22 18:38:41.0 -0600
+++ guile-1.8.6/libguile/modules.c 2008-12-22 20:22:19.0 -0600
@@ -555,11 +555,16 @@ scm_c_define (const char *name, SCM valu
return scm_define (scm_from_locale_symbol (name), value);
}
+scm_i_pthread_mutex_t scm_i_define_mutex;
+
SCM
scm_define (SCM sym, SCM value)
{
- SCM var =
+ SCM var;
+ scm_pthread_mutex_lock(scm_i_define_mutex);
+ var =
scm_sym2var (sym, scm_current_module_lookup_closure (), SCM_BOOL_T);
+ scm_i_pthread_mutex_unlock(scm_i_define_mutex);
SCM_VARIABLE_SET (var, value);
return var;
}
@@ -651,6 +656,8 @@ void
scm_init_modules ()
{
#include libguile/modules.x
+ scm_i_pthread_mutex_init (scm_i_define_mutex, NULL);
+
module_make_local_var_x_var = scm_c_define (module-make-local-var!,
SCM_UNDEFINED);
scm_tc16_eval_closure = scm_make_smob_type (eval-closure, 0);
(file #17118)
___
Additional Item Attachment:
File name: define-race.patch Size:1 KB
___
Reply to this item at:
http://savannah.gnu.org/bugs/?24867
___
Message sent via/by Savannah
http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Additional Item Attachment, bug #24867 (project guile): File name: define-race.c Size:2 KB ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #3, bug #24867 (project guile): Note that the attached test case shows 4 distinct behaviours: 1) runs fine, exiting normally, w/o any errors 2) Spewing messages such as: ERROR: Unbound variable: x2-347 ERROR: Unbound variable: x2-347 ERROR: Unbound variable: x2-347 ERROR: Unbound variable: define ERROR: Unbound variable: x1-2525 ERROR: Unbound variable: x1-2525 ERROR: Unbound variable: x1-2525 ERROR: Unbound variable: x1-2525 ERROR: Unbound variable: x1-2525 ERROR: Unbound variable: x1-2525 the numbers being different each time, of course, but then exiting normally. 3) Deadlocking in garbage collection, with all four threads stuck in #5 0xf7f01f6b in scm_gc_for_newcell (freelist=0xf7f8c8ec, free_cells=0x90c810c) at gc.c:484 (this may be preceeded by the prints above) 4) Segfault: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0xf74c5b90 (LWP 30649)] 0xf7f426ad in scm_assert_smob_type (tag=639, val=0x0) at smob.c:63 63if (!SCM_SMOB_PREDICATE (tag, val)) (gdb) bt #0 0xf7f426ad in scm_assert_smob_type (tag=639, val=0x0) at smob.c:63 #1 0xf7f0b4c5 in scm_make_dynamic_state (parent=0x0) at fluids.c:508 #2 0xf7f62bdf in guilify_self_2 (parent=0x0) at threads.c:508 #3 0xf7f64992 in scm_i_init_thread_for_guile (base=0xf74c5388, parent=0x0) at threads.c:611 #4 0xf7f649c5 in scm_i_with_guile_and_parent ( func=0x8048754 guile_mode_definer, data=0xffaf0cbc, parent=0x0) at threads.c:743 #5 0xf7f64ace in scm_with_guile (func=0x8048754 guile_mode_definer, data=0xffaf0cbc) at threads.c:732 #6 0x080488a8 in definer () #7 0xf7fa14fb in start_thread () from /lib/tls/i686/cmov/libpthread.so.0 #8 0xf7e45e5e in clone () from /lib/tls/i686/cmov/libc.so.6 I'm guessing that the use of immutable vlists, as proposed by Ludo, for use in the module-obarray per mail discussion, would solve all of the above. ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
[bug #24867] `define' should be thread-safe
Follow-up Comment #1, bug #24867 (project guile): Gmane appears to be broken. Alternate URL: http://lists.gnu.org/archive/html/guile-devel/2008-11/threads.html#00055 . ___ Reply to this item at: http://savannah.gnu.org/bugs/?24867 ___ Message sent via/by Savannah http://savannah.gnu.org/
