* Thread-unsafe initialization problems in Guile @ 2012-11-29 18:54 Mark H Weaver 2012-11-29 20:05 ` Ludovic Courtès 0 siblings, 1 reply; 16+ messages in thread From: Mark H Weaver @ 2012-11-29 18:54 UTC (permalink / raw) To: guile-devel; +Cc: Ludovic Courtès It would be good to fix these before 2.0.7 is released, but I'm a bit uncertain of how best to do so, and I don't have time at the moment. ./posix.c:1375: make_rw_port = scm_c_private_variable ("ice-9 popen", ./debug.c:217: local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); ./strports.c:541: eval_string = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2012-11-29 18:54 Thread-unsafe initialization problems in Guile Mark H Weaver @ 2012-11-29 20:05 ` Ludovic Courtès 2012-11-29 22:42 ` Mark H Weaver 0 siblings, 1 reply; 16+ messages in thread From: Ludovic Courtès @ 2012-11-29 20:05 UTC (permalink / raw) To: Mark H Weaver; +Cc: guile-devel Hi, Mark H Weaver <mhw@netris.org> skribis: > It would be good to fix these before 2.0.7 is released, but I'm a bit > uncertain of how best to do so, and I don't have time at the moment. > > ./posix.c:1375: make_rw_port = scm_c_private_variable ("ice-9 popen", > ./debug.c:217: local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); > ./strports.c:541: eval_string = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); I’m actually not sure what you mean here. Anyway, I think we ought to release 2.0.7 now. And yes, there will be a 2.0.8 with a number of additional fixes. :-) WDYT? Ludo’. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2012-11-29 20:05 ` Ludovic Courtès @ 2012-11-29 22:42 ` Mark H Weaver 2013-01-21 20:04 ` Andy Wingo 0 siblings, 1 reply; 16+ messages in thread From: Mark H Weaver @ 2012-11-29 22:42 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guile-devel ludo@gnu.org (Ludovic Courtès) writes: > Mark H Weaver <mhw@netris.org> skribis: > >> It would be good to fix these before 2.0.7 is released, but I'm a bit >> uncertain of how best to do so, and I don't have time at the moment. >> >> ./posix.c:1375: make_rw_port = scm_c_private_variable ("ice-9 popen", >> ./debug.c:217: local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); >> ./strports.c:541: eval_string = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); > > I’m actually not sure what you mean here. Those are three bugs where a global static variable is lazily initialized in a way that is not thread-safe. The one in strports.c actually caused a reproducible crash bug for a user who created multiple threads, each of which called scm_eval_string. Consider the bug in scm_local_eval, which I'm ashamed to say was written by me before I educated myself on weakly-ordered memory models: SCM scm_local_eval (SCM exp, SCM env) { static SCM local_eval_var = SCM_BOOL_F; if (scm_is_false (local_eval_var)) local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); return scm_call_2 (SCM_VARIABLE_REF (local_eval_var), exp, env); } The problem is that it's possible for a thread to see a non-#f value for 'local_eval_var' before it sees the memory it points to properly initialized. > Anyway, I think we ought to release 2.0.7 now. And yes, there will be a > 2.0.8 with a number of additional fixes. :-) Okay, that's fine. We can fix these for 2.0.8. Thanks, Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2012-11-29 22:42 ` Mark H Weaver @ 2013-01-21 20:04 ` Andy Wingo 2013-01-23 17:50 ` Mark H Weaver 0 siblings, 1 reply; 16+ messages in thread From: Andy Wingo @ 2013-01-21 20:04 UTC (permalink / raw) To: Mark H Weaver; +Cc: Ludovic Courtès, guile-devel Hi, On Thu 29 Nov 2012 23:42, Mark H Weaver <mhw@netris.org> writes: > SCM > scm_local_eval (SCM exp, SCM env) > { > static SCM local_eval_var = SCM_BOOL_F; > > if (scm_is_false (local_eval_var)) > local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); > > return scm_call_2 (SCM_VARIABLE_REF (local_eval_var), exp, env); > } > > The problem is that it's possible for a thread to see a non-#f value for > 'local_eval_var' before it sees the memory it points to properly > initialized. scm_c_public_variable is not idempotent? If it is, where is the problem? Andy -- http://wingolog.org/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2013-01-21 20:04 ` Andy Wingo @ 2013-01-23 17:50 ` Mark H Weaver 2013-01-23 18:25 ` Mark H Weaver 0 siblings, 1 reply; 16+ messages in thread From: Mark H Weaver @ 2013-01-23 17:50 UTC (permalink / raw) To: Andy Wingo; +Cc: Ludovic Courtès, guile-devel Hi Andy, Andy Wingo <wingo@pobox.com> writes: > On Thu 29 Nov 2012 23:42, Mark H Weaver <mhw@netris.org> writes: > >> SCM >> scm_local_eval (SCM exp, SCM env) >> { >> static SCM local_eval_var = SCM_BOOL_F; >> >> if (scm_is_false (local_eval_var)) >> local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); >> >> return scm_call_2 (SCM_VARIABLE_REF (local_eval_var), exp, env); >> } >> >> The problem is that it's possible for a thread to see a non-#f value for >> 'local_eval_var' before it sees the memory it points to properly >> initialized. > > scm_c_public_variable is not idempotent? It is idempotent as far as I know, but that's not the problem. The problem is that in modern weakly-ordered memory models, there is no guarantee that thread B will see writes in the same order as they were performed by thread A. For example, if (ice-9 local-eval) has not yet been loaded when thread A calls 'scm_local_eval', then thread A will load the (ice-9 local-eval) module, allocate and initialize the associated SCM variable objects, and finally write 'local_eval_var' to point to one of these variable objects. However, in the absence of thread synchronization, thread B may see 'local_eval_var' set to a non-#f value before it sees the writes that initialized the variable object to which 'local_eval_var' points. In other words, thread B may end up reading a SCM variable object that has not yet been initialized in its timeline. For a good introduction to what is needed to write robust multithreaded code on modern weakly-ordered memory architectures, I recommend the following article: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2480.html Regards, Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2013-01-23 17:50 ` Mark H Weaver @ 2013-01-23 18:25 ` Mark H Weaver 2013-02-28 21:07 ` Mark H Weaver 0 siblings, 1 reply; 16+ messages in thread From: Mark H Weaver @ 2013-01-23 18:25 UTC (permalink / raw) To: Andy Wingo; +Cc: Ludovic Courtès, guile-devel I wrote: > For a good introduction to what is needed to write robust multithreaded > code on modern weakly-ordered memory architectures, I recommend the > following article: > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2480.html and if you want a deeper understanding of what's going on in the hardware to cause these problems, see Paul McKenney's paper "Memory Barriers: a Hardware View for Software Hackers": http://www.rdrop.com/users/paulmck/scalability/paper/whymb.2010.06.07c.pdf Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2013-01-23 18:25 ` Mark H Weaver @ 2013-02-28 21:07 ` Mark H Weaver 2013-02-28 23:20 ` [PATCH] Fix thread-unsafe lazy initializations Mark H Weaver 2013-03-05 20:00 ` Thread-unsafe initialization problems in Guile Noah Lavine 0 siblings, 2 replies; 16+ messages in thread From: Mark H Weaver @ 2013-02-28 21:07 UTC (permalink / raw) To: Andy Wingo; +Cc: Ludovic Courtès, guile-devel Here's an article from a reputable source that backs up my claim that the lazy-initialization pattern that we use in a few places is not thread-safe: http://www.ibm.com/developerworks/java/library/j-dcl/index.html Here are the thread-unsafe lazy initializations that I'm aware of: ./posix.c:1375: make_rw_port = scm_c_private_variable ("ice-9 popen", ./debug.c:217: local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); ./strports.c:541: eval_string = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); In each of these cases, we have two options: (1) synchronize on every access of the lazily-initialized variable (including reads), or (2) abandon lazy initialization. Thoughts? Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] Fix thread-unsafe lazy initializations 2013-02-28 21:07 ` Mark H Weaver @ 2013-02-28 23:20 ` Mark H Weaver 2013-03-01 9:23 ` Ludovic Courtès 2013-03-05 20:00 ` Thread-unsafe initialization problems in Guile Noah Lavine 1 sibling, 1 reply; 16+ messages in thread From: Mark H Weaver @ 2013-02-28 23:20 UTC (permalink / raw) To: Andy Wingo; +Cc: Ludovic Courtès, guile-devel [-- Attachment #1: Type: text/plain, Size: 79 bytes --] Here's my proposed patch to fix these problems. What do you think? Mark [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: [PATCH] Fix thread-unsafe lazy initializations --] [-- Type: text/x-diff, Size: 4085 bytes --] From dadcb1512569c1be039fc75f0a2967e370939e42 Mon Sep 17 00:00:00 2001 From: Mark H Weaver <mhw@netris.org> Date: Thu, 28 Feb 2013 17:56:58 -0500 Subject: [PATCH] Fix thread-unsafe lazy initializations. * libguile/debug.c (scm_local_eval): libguile/ports.c (scm_current_warning_port): libguile/strports.c (scm_eval_string_in_module): Perform lazy-initialization while holding a mutex. Use SCM_UNDEFINED as the uninitialized value. Use 'scm_c_*_variable'. * doc/ref/api-modules.texi (Accessing Modules from C): Fix 'my_eval_string' example to be thread-safe. --- doc/ref/api-modules.texi | 13 +++++++------ libguile/debug.c | 8 ++++++-- libguile/ports.c | 12 ++++++++---- libguile/strports.c | 9 ++++++--- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/doc/ref/api-modules.texi b/doc/ref/api-modules.texi index 17ab462..3d111c4 100644 --- a/doc/ref/api-modules.texi +++ b/doc/ref/api-modules.texi @@ -942,14 +942,15 @@ the @var{name} is not bound in the module, signals an error. Returns a variable, always. @example -SCM my_eval_string (SCM str) -@{ - static SCM eval_string_var = SCM_BOOL_F; +static SCM eval_string_var; - if (scm_is_false (eval_string_var)) - eval_string_var = - scm_c_public_lookup ("ice-9 eval-string", "eval-string"); +void my_init (void) +@{ + eval_string_var = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); +@} +SCM my_eval_string (SCM str) +@{ return scm_call_1 (scm_variable_ref (eval_string_var), str); @} @end example diff --git a/libguile/debug.c b/libguile/debug.c index b1a90d8..9e6328b 100644 --- a/libguile/debug.c +++ b/libguile/debug.c @@ -211,10 +211,14 @@ SCM_DEFINE (scm_debug_hang, "debug-hang", 0, 1, 0, SCM scm_local_eval (SCM exp, SCM env) { - static SCM local_eval_var = SCM_BOOL_F; + static SCM local_eval_var = SCM_UNDEFINED; + static scm_i_pthread_mutex_t local_eval_var_mutex + = SCM_I_PTHREAD_MUTEX_INITIALIZER; - if (scm_is_false (local_eval_var)) + scm_i_scm_pthread_mutex_lock (&local_eval_var_mutex); + if (SCM_UNBNDP (local_eval_var)) local_eval_var = scm_c_public_variable ("ice-9 local-eval", "local-eval"); + scm_i_pthread_mutex_unlock (&local_eval_var_mutex); return scm_call_2 (SCM_VARIABLE_REF (local_eval_var), exp, env); } diff --git a/libguile/ports.c b/libguile/ports.c index 55808e2..8737a76 100644 --- a/libguile/ports.c +++ b/libguile/ports.c @@ -418,10 +418,14 @@ SCM_DEFINE (scm_current_error_port, "current-error-port", 0, 0, 0, SCM scm_current_warning_port (void) { - static SCM cwp_var = SCM_BOOL_F; - - if (scm_is_false (cwp_var)) - cwp_var = scm_c_private_lookup ("guile", "current-warning-port"); + static SCM cwp_var = SCM_UNDEFINED; + static scm_i_pthread_mutex_t cwp_var_mutex + = SCM_I_PTHREAD_MUTEX_INITIALIZER; + + scm_i_scm_pthread_mutex_lock (&cwp_var_mutex); + if (SCM_UNBNDP (cwp_var)) + cwp_var = scm_c_private_variable ("guile", "current-warning-port"); + scm_i_pthread_mutex_unlock (&cwp_var_mutex); return scm_call_0 (scm_variable_ref (cwp_var)); } diff --git a/libguile/strports.c b/libguile/strports.c index 14cc93f..d1b293c 100644 --- a/libguile/strports.c +++ b/libguile/strports.c @@ -534,13 +534,16 @@ SCM_DEFINE (scm_eval_string_in_module, "eval-string", 1, 1, 0, "procedure returns.") #define FUNC_NAME s_scm_eval_string_in_module { - static SCM eval_string = SCM_BOOL_F, k_module = SCM_BOOL_F; + static SCM eval_string = SCM_UNDEFINED, k_module = SCM_UNDEFINED; + static scm_i_pthread_mutex_t init_mutex = SCM_I_PTHREAD_MUTEX_INITIALIZER; - if (scm_is_false (eval_string)) + scm_i_scm_pthread_mutex_lock (&init_mutex); + if (SCM_UNBNDP (eval_string)) { - eval_string = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); + eval_string = scm_c_public_variable ("ice-9 eval-string", "eval-string"); k_module = scm_from_locale_keyword ("module"); } + scm_i_pthread_mutex_unlock (&init_mutex); if (SCM_UNBNDP (module)) module = scm_current_module (); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] Fix thread-unsafe lazy initializations 2013-02-28 23:20 ` [PATCH] Fix thread-unsafe lazy initializations Mark H Weaver @ 2013-03-01 9:23 ` Ludovic Courtès 2013-03-05 18:53 ` Mark H Weaver 0 siblings, 1 reply; 16+ messages in thread From: Ludovic Courtès @ 2013-03-01 9:23 UTC (permalink / raw) To: Mark H Weaver; +Cc: Andy Wingo, guile-devel Mark H Weaver <mhw@netris.org> skribis: > From dadcb1512569c1be039fc75f0a2967e370939e42 Mon Sep 17 00:00:00 2001 > From: Mark H Weaver <mhw@netris.org> > Date: Thu, 28 Feb 2013 17:56:58 -0500 > Subject: [PATCH] Fix thread-unsafe lazy initializations. > > * libguile/debug.c (scm_local_eval): > libguile/ports.c (scm_current_warning_port): > libguile/strports.c (scm_eval_string_in_module): Perform > lazy-initialization while holding a mutex. Use SCM_UNDEFINED as the > uninitialized value. Use 'scm_c_*_variable'. > > * doc/ref/api-modules.texi (Accessing Modules from C): Fix > 'my_eval_string' example to be thread-safe. Looks good to me. > --- a/doc/ref/api-modules.texi > +++ b/doc/ref/api-modules.texi > @@ -942,14 +942,15 @@ the @var{name} is not bound in the module, signals an error. Returns a > variable, always. > > @example > -SCM my_eval_string (SCM str) > -@{ > - static SCM eval_string_var = SCM_BOOL_F; > +static SCM eval_string_var; > > - if (scm_is_false (eval_string_var)) > - eval_string_var = > - scm_c_public_lookup ("ice-9 eval-string", "eval-string"); > +void my_init (void) > +@{ > + eval_string_var = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); > +@} > > +SCM my_eval_string (SCM str) > +@{ > return scm_call_1 (scm_variable_ref (eval_string_var), str); > @} > @end example The doc should say something about ‘my_init’, and perhaps mention locking? Ludo’. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Fix thread-unsafe lazy initializations 2013-03-01 9:23 ` Ludovic Courtès @ 2013-03-05 18:53 ` Mark H Weaver 2013-03-05 20:45 ` Ludovic Courtès 0 siblings, 1 reply; 16+ messages in thread From: Mark H Weaver @ 2013-03-05 18:53 UTC (permalink / raw) To: Ludovic Courtès; +Cc: Andy Wingo, guile-devel Hi Ludovic, ludo@gnu.org (Ludovic Courtès) writes: > Mark H Weaver <mhw@netris.org> skribis: > >> --- a/doc/ref/api-modules.texi >> +++ b/doc/ref/api-modules.texi >> @@ -942,14 +942,15 @@ the @var{name} is not bound in the module, signals an error. Returns a >> variable, always. >> >> @example >> -SCM my_eval_string (SCM str) >> -@{ >> - static SCM eval_string_var = SCM_BOOL_F; >> +static SCM eval_string_var; >> >> - if (scm_is_false (eval_string_var)) >> - eval_string_var = >> - scm_c_public_lookup ("ice-9 eval-string", "eval-string"); >> +void my_init (void) >> +@{ >> + eval_string_var = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); >> +@} >> >> +SCM my_eval_string (SCM str) >> +@{ >> return scm_call_1 (scm_variable_ref (eval_string_var), str); >> @} >> @end example > > The doc should say something about ‘my_init’, and perhaps mention > locking? Can you suggest some text? 'my_init' seems obvious to me, and given that this is just an example for 'scm_c_*_lookup', it seems out of place to talk about general locking issues here, but feel free to propose some text to go here. Thanks, Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Fix thread-unsafe lazy initializations 2013-03-05 18:53 ` Mark H Weaver @ 2013-03-05 20:45 ` Ludovic Courtès 2013-03-05 21:10 ` Mark H Weaver 0 siblings, 1 reply; 16+ messages in thread From: Ludovic Courtès @ 2013-03-05 20:45 UTC (permalink / raw) To: Mark H Weaver; +Cc: Andy Wingo, guile-devel Mark H Weaver <mhw@netris.org> skribis: > ludo@gnu.org (Ludovic Courtès) writes: > >> Mark H Weaver <mhw@netris.org> skribis: >> >>> --- a/doc/ref/api-modules.texi >>> +++ b/doc/ref/api-modules.texi >>> @@ -942,14 +942,15 @@ the @var{name} is not bound in the module, signals an error. Returns a >>> variable, always. >>> >>> @example >>> -SCM my_eval_string (SCM str) >>> -@{ >>> - static SCM eval_string_var = SCM_BOOL_F; >>> +static SCM eval_string_var; >>> >>> - if (scm_is_false (eval_string_var)) >>> - eval_string_var = >>> - scm_c_public_lookup ("ice-9 eval-string", "eval-string"); >>> +void my_init (void) >>> +@{ >>> + eval_string_var = scm_c_public_lookup ("ice-9 eval-string", "eval-string"); >>> +@} >>> >>> +SCM my_eval_string (SCM str) >>> +@{ >>> return scm_call_1 (scm_variable_ref (eval_string_var), str); >>> @} >>> @end example >> >> The doc should say something about ‘my_init’, and perhaps mention >> locking? > > Can you suggest some text? 'my_init' seems obvious to me, and given > that this is just an example for 'scm_c_*_lookup', it seems out of place > to talk about general locking issues here, but feel free to propose some > text to go here. Agreed. Perhaps just something like: “Note that the program should ensure that ‘my_init’ is called only once, and in a thread-safe way.” Ludo’. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Fix thread-unsafe lazy initializations 2013-03-05 20:45 ` Ludovic Courtès @ 2013-03-05 21:10 ` Mark H Weaver 0 siblings, 0 replies; 16+ messages in thread From: Mark H Weaver @ 2013-03-05 21:10 UTC (permalink / raw) To: Ludovic Courtès; +Cc: Andy Wingo, guile-devel ludo@gnu.org (Ludovic Courtès) writes: > Agreed. Perhaps just something like: “Note that the program should > ensure that ‘my_init’ is called only once, and in a thread-safe way.” Okay, I went ahead and pushed it to stable-2.0, with the code comment: "It is important that the call to 'my_init' happens-before all calls to 'my_eval_string'." Thanks, Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2013-02-28 21:07 ` Mark H Weaver 2013-02-28 23:20 ` [PATCH] Fix thread-unsafe lazy initializations Mark H Weaver @ 2013-03-05 20:00 ` Noah Lavine 2013-03-06 3:24 ` Mark H Weaver 1 sibling, 1 reply; 16+ messages in thread From: Noah Lavine @ 2013-03-05 20:00 UTC (permalink / raw) To: Mark H Weaver; +Cc: Andy Wingo, Ludovic Courtès, guile-devel [-- Attachment #1: Type: text/plain, Size: 1145 bytes --] I've only read the most recent article you posted, but if I understand correctly, there is a third option: (3) somehow find a way to generate a portable memory barrier instruction. Is that currently possible? I'm not sure that it is. Probably option (2) is best if we can do it. Noah On Thu, Feb 28, 2013 at 4:07 PM, Mark H Weaver <mhw@netris.org> wrote: > Here's an article from a reputable source that backs up my claim that > the lazy-initialization pattern that we use in a few places is not > thread-safe: > > http://www.ibm.com/developerworks/java/library/j-dcl/index.html > > Here are the thread-unsafe lazy initializations that I'm aware of: > > ./posix.c:1375: make_rw_port = scm_c_private_variable ("ice-9 > popen", > ./debug.c:217: local_eval_var = scm_c_public_variable ("ice-9 > local-eval", "local-eval"); > ./strports.c:541: eval_string = scm_c_public_lookup ("ice-9 > eval-string", "eval-string"); > > In each of these cases, we have two options: (1) synchronize on every > access of the lazily-initialized variable (including reads), or (2) > abandon lazy initialization. > > Thoughts? > Mark > > [-- Attachment #2: Type: text/html, Size: 1771 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2013-03-05 20:00 ` Thread-unsafe initialization problems in Guile Noah Lavine @ 2013-03-06 3:24 ` Mark H Weaver 2013-03-06 3:39 ` Noah Lavine 0 siblings, 1 reply; 16+ messages in thread From: Mark H Weaver @ 2013-03-06 3:24 UTC (permalink / raw) To: Noah Lavine; +Cc: Andy Wingo, Ludovic Courtès, guile-devel Hi Noah, I wrote: > In each of these cases, we have two options: (1) synchronize on every > access of the lazily-initialized variable (including reads), or (2) > abandon lazy initialization. Noah Lavine <noah.b.lavine@gmail.com> writes: > I've only read the most recent article you posted, but if I understand > correctly, there is a third option: (3) somehow find a way to generate > a portable memory barrier instruction. Is that currently possible? If we were to do that, we'd have to add memory barriers in two places: (1) after writing to the lazily-initialized variable, and (2) before reading from it. While memory barriers are somewhat more efficient than mutexes, they are still very expensive. As for portability, C11 is the first C standard to support memory barriers. For now, our best bet would probably be to use libatomic_ops, which is also used by libgc. > Probably option (2) is best if we can do it. Agreed. Unfortunately, in these cases option (2) would significantly increase the number of modules that need to be loaded at initialization time. That's why I reluctantly chose option (1). Regards, Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2013-03-06 3:24 ` Mark H Weaver @ 2013-03-06 3:39 ` Noah Lavine 2013-03-06 3:50 ` Mark H Weaver 0 siblings, 1 reply; 16+ messages in thread From: Noah Lavine @ 2013-03-06 3:39 UTC (permalink / raw) To: Mark H Weaver; +Cc: Andy Wingo, Ludovic Courtès, guile-devel [-- Attachment #1: Type: text/plain, Size: 1833 bytes --] Hello, On Tue, Mar 5, 2013 at 10:24 PM, Mark H Weaver <mhw@netris.org> wrote: > Hi Noah, > > Noah Lavine <noah.b.lavine@gmail.com> writes: > > I've only read the most recent article you posted, but if I understand > > correctly, there is a third option: (3) somehow find a way to generate > > a portable memory barrier instruction. Is that currently possible? > > If we were to do that, we'd have to add memory barriers in two places: > (1) after writing to the lazily-initialized variable, and (2) before > reading from it. While memory barriers are somewhat more efficient than > mutexes, they are still very expensive. > I'm not sure I understand the issue, but I think I was imagining something like if (variable == SCM_BOOL_F) { acquire_mutex(var_mutex); if (variable == SCM_BOOL_F) { variable = initialize_variable(); memory_barrier(); } release_mutex(var_mutex); } That's really just a normal locking scheme with an added memory barrier to make sure that all threads see an update after the first thread updates the variable. Would that work? > As for portability, C11 is the first C standard to support memory > barriers. For now, our best bet would probably be to use libatomic_ops, > which is also used by libgc. > That means we already depend on libatomic_ops, which is good. However, I see that the website for that library recommends using C11 instead. But I really doubt that this issue is a big enough justification to use either libatomic_ops or C11. > > Probably option (2) is best if we can do it. > > Agreed. Unfortunately, in these cases option (2) would significantly > increase the number of modules that need to be loaded at initialization > time. That's why I reluctantly chose option (1). > That makes sense. Thanks a lot for helping me understand these memory issues. Noah [-- Attachment #2: Type: text/html, Size: 2829 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Thread-unsafe initialization problems in Guile 2013-03-06 3:39 ` Noah Lavine @ 2013-03-06 3:50 ` Mark H Weaver 0 siblings, 0 replies; 16+ messages in thread From: Mark H Weaver @ 2013-03-06 3:50 UTC (permalink / raw) To: Noah Lavine; +Cc: Andy Wingo, Ludovic Courtès, guile-devel Noah Lavine <noah.b.lavine@gmail.com> writes: > I'm not sure I understand the issue, but I think I was imagining > something like > > if (variable == SCM_BOOL_F) { > acquire_mutex(var_mutex); > if (variable == SCM_BOOL_F) { > variable = initialize_variable(); > memory_barrier(); > } > release_mutex(var_mutex); > } > > That's really just a normal locking scheme with an added memory > barrier to make sure that all threads see an update after the first > thread updates the variable. Would that work? No. There's no portable memory barrier that can be used only by the writer of a shared variable. The reader must also do synchronization in order to prove the "happens-before" relationship. Regards, Mark ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-03-06 3:50 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-29 18:54 Thread-unsafe initialization problems in Guile Mark H Weaver 2012-11-29 20:05 ` Ludovic Courtès 2012-11-29 22:42 ` Mark H Weaver 2013-01-21 20:04 ` Andy Wingo 2013-01-23 17:50 ` Mark H Weaver 2013-01-23 18:25 ` Mark H Weaver 2013-02-28 21:07 ` Mark H Weaver 2013-02-28 23:20 ` [PATCH] Fix thread-unsafe lazy initializations Mark H Weaver 2013-03-01 9:23 ` Ludovic Courtès 2013-03-05 18:53 ` Mark H Weaver 2013-03-05 20:45 ` Ludovic Courtès 2013-03-05 21:10 ` Mark H Weaver 2013-03-05 20:00 ` Thread-unsafe initialization problems in Guile Noah Lavine 2013-03-06 3:24 ` Mark H Weaver 2013-03-06 3:39 ` Noah Lavine 2013-03-06 3:50 ` Mark H Weaver
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).