[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/
