* [bug #24867] `define' should be thread-safe @ 2008-11-18 10:18 Ludovic Courtès 2008-11-18 13:27 ` Ludovic Courtès 0 siblings, 1 reply; 13+ messages in thread From: Ludovic Courtès @ 2008-11-18 10:18 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, linasvepstas, bug-guile URL: <http://savannah.gnu.org/bugs/?24867> Summary: `define' should be thread-safe Project: Guile Submitted by: civodul Submitted on: Tue 18 Nov 2008 10:18:28 AM GMT Category: None Severity: 3 - Normal Item Group: None Status: None Privacy: Public Assigned to: None Open/Closed: Open Discussion Lock: Any _______________________________________________________ Details: `define' uses a hash table behind the scenes. The hash table itself is not thread-safe, but `define' should be thread-safe, that is, by synchronizing accesses to the underlying hash table. See http://thread.gmane.org/gmane.lisp.guile.devel/7851/focus=7858 for the original report. Thanks, Ludo'. _______________________________________________________ Reply to this item at: <http://savannah.gnu.org/bugs/?24867> _______________________________________________ Message sent via/by Savannah http://savannah.gnu.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-11-18 10:18 [bug #24867] `define' should be thread-safe Ludovic Courtès @ 2008-11-18 13:27 ` Ludovic Courtès 2008-12-23 2:36 ` Linas Vepstas 0 siblings, 1 reply; 13+ messages in thread From: Ludovic Courtès @ 2008-11-18 13:27 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, linasvepstas, bug-guile 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-11-18 13:27 ` Ludovic Courtès @ 2008-12-23 2:36 ` Linas Vepstas 2008-12-23 2:50 ` Linas Vepstas 0 siblings, 1 reply; 13+ messages in thread From: Linas Vepstas @ 2008-12-23 2:36 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, bug-guile 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 <linasvepstas@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.000000000 -0600 +++ guile-1.8.6/libguile/modules.c 2008-12-22 20:22:19.000000000 -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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-12-23 2:36 ` Linas Vepstas @ 2008-12-23 2:50 ` Linas Vepstas 2008-12-23 3:06 ` Linas Vepstas 0 siblings, 1 reply; 13+ messages in thread From: Linas Vepstas @ 2008-12-23 2:50 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, bug-guile 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-12-23 2:50 ` Linas Vepstas @ 2008-12-23 3:06 ` Linas Vepstas 2008-12-23 15:21 ` Ludovic Courtès 0 siblings, 1 reply; 13+ messages in thread From: Linas Vepstas @ 2008-12-23 3:06 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, bug-guile 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 63 if (!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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-12-23 3:06 ` Linas Vepstas @ 2008-12-23 15:21 ` Ludovic Courtès 2008-12-23 18:28 ` Linas Vepstas 0 siblings, 1 reply; 13+ messages in thread From: Ludovic Courtès @ 2008-12-23 15:21 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, bug-guile 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-12-23 15:21 ` Ludovic Courtès @ 2008-12-23 18:28 ` Linas Vepstas 2008-12-23 19:10 ` Linas Vepstas 0 siblings, 1 reply; 13+ messages in thread From: Linas Vepstas @ 2008-12-23 18:28 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, bug-guile 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-12-23 18:28 ` Linas Vepstas @ 2008-12-23 19:10 ` Linas Vepstas 2010-09-20 9:28 ` Kourtney Keese 0 siblings, 1 reply; 13+ messages in thread From: Linas Vepstas @ 2008-12-23 19:10 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, bug-guile 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2008-12-23 19:10 ` Linas Vepstas @ 2010-09-20 9:28 ` Kourtney Keese 2011-08-06 9:08 ` JackBenny 0 siblings, 1 reply; 13+ messages in thread From: Kourtney Keese @ 2010-09-20 9:28 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, Kourtney Keese, bug-guile Follow-up Comment #7, bug #24867 (project guile): thans for sharing.its very usefull. <a href="http://www.cheap-poloshirts.com">lacoste miami</a> _______________________________________________________ Reply to this item at: <http://savannah.gnu.org/bugs/?24867> _______________________________________________ Message sent via/by Savannah http://savannah.gnu.org/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2010-09-20 9:28 ` Kourtney Keese @ 2011-08-06 9:08 ` JackBenny 2011-08-06 9:08 ` JackBenny 0 siblings, 1 reply; 13+ messages in thread From: JackBenny @ 2011-08-06 9:08 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, -Deleted Account- 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2011-08-06 9:08 ` JackBenny @ 2011-08-06 9:08 ` JackBenny 2011-08-06 9:09 ` JackBenny 0 siblings, 1 reply; 13+ messages in thread From: JackBenny @ 2011-08-06 9:08 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, -Deleted Account- 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2011-08-06 9:08 ` JackBenny @ 2011-08-06 9:09 ` JackBenny 2011-08-06 9:09 ` JackBenny 0 siblings, 1 reply; 13+ messages in thread From: JackBenny @ 2011-08-06 9:09 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, -Deleted Account- 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [bug #24867] `define' should be thread-safe 2011-08-06 9:09 ` JackBenny @ 2011-08-06 9:09 ` JackBenny 0 siblings, 0 replies; 13+ messages in thread From: JackBenny @ 2011-08-06 9:09 UTC (permalink / raw) To: Andy Wingo, Ludovic Courtès, Linas Vepstas, -Deleted Account- 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/ ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-08-06 9:09 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-11-18 10:18 [bug #24867] `define' should be thread-safe Ludovic Courtès 2008-11-18 13:27 ` Ludovic Courtès 2008-12-23 2:36 ` Linas Vepstas 2008-12-23 2:50 ` Linas Vepstas 2008-12-23 3:06 ` Linas Vepstas 2008-12-23 15:21 ` Ludovic Courtès 2008-12-23 18:28 ` Linas Vepstas 2008-12-23 19:10 ` Linas Vepstas 2010-09-20 9:28 ` Kourtney Keese 2011-08-06 9:08 ` JackBenny 2011-08-06 9:08 ` JackBenny 2011-08-06 9:09 ` JackBenny 2011-08-06 9:09 ` JackBenny
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).