unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* Re: [Guile-commits] GNU Guile branch, master, updated. v2.1.0-213-gb43e81d
       [not found] <E1VRpou-0002qh-2M@vcs.savannah.gnu.org>
@ 2013-10-04  4:20 ` Mark H Weaver
  2013-10-05 11:22   ` Andy Wingo
  0 siblings, 1 reply; 2+ messages in thread
From: Mark H Weaver @ 2013-10-04  4:20 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Hi Andy,

"Andy Wingo" <wingo@pobox.com> writes:

> diff --git a/libguile/programs.c b/libguile/programs.c
> index 37130d0..5039d2a 100644
> --- a/libguile/programs.c
> +++ b/libguile/programs.c
> @@ -297,7 +297,7 @@ SCM_DEFINE (scm_program_bindings, "program-bindings", 1, 0, 0,
>  }
>  #undef FUNC_NAME
>  
> -SCM_DEFINE (scm_program_sources, "program-sources", 1, 0, 0,
> +SCM_DEFINE (scm_program_sources, "%program-sources", 1, 0, 0,
>  	    (SCM program),
>  	    "")
>  #define FUNC_NAME s_scm_program_sources
> @@ -365,32 +365,24 @@ scm_i_program_properties (SCM program)
>  }
>  #undef FUNC_NAME
>  
> -static SCM
> -program_source (SCM program, size_t ip, SCM sources)
> +SCM
> +scm_program_source (SCM program, SCM ip, SCM sources)
>  {
> -  SCM source = SCM_BOOL_F;
> +  static SCM program_source = SCM_BOOL_F;
>  
> -  while (!scm_is_null (sources)
> -         && scm_to_size_t (scm_caar (sources)) <= ip)
> -    {
> -      source = scm_car (sources);
> -      sources = scm_cdr (sources);
> -    }
> -  
> -  return source; /* (addr . (filename . (line . column))) */
> -}
> +  if (scm_is_false (program_source)) {
> +    if (!scm_module_system_booted_p)
> +      return SCM_BOOL_F;
> +
> +    program_source =
> +      scm_c_private_variable ("system vm program", "program-source");
> +  }

This lazy initialization pattern is not safe on modern weakly ordered
memory architectures.  I've already raised this issue on the mailing
list more than once, and I've been trying to fix these bugs wherever I
can find them, but they are non-trivial to search for.  Please do not
add more.

We need to either use a mutex to guard both reads and writes of
'program_source' (including the "scm_is_false (program_source)" check),
or we need to arrange for 'program_source' to be initialized during
guile's initialization before any other threads are created.

     Mark



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Guile-commits] GNU Guile branch, master, updated. v2.1.0-213-gb43e81d
  2013-10-04  4:20 ` [Guile-commits] GNU Guile branch, master, updated. v2.1.0-213-gb43e81d Mark H Weaver
@ 2013-10-05 11:22   ` Andy Wingo
  0 siblings, 0 replies; 2+ messages in thread
From: Andy Wingo @ 2013-10-05 11:22 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guile-devel

Hi,

On Fri 04 Oct 2013 06:20, Mark H Weaver <mhw@netris.org> writes:

> This lazy initialization pattern is not safe on modern weakly ordered
> memory architectures.  I've already raised this issue on the mailing
> list more than once, and I've been trying to fix these bugs wherever I
> can find them, but they are non-trivial to search for.  Please do not
> add more.
>
> We need to either use a mutex to guard both reads and writes of
> 'program_source' (including the "scm_is_false (program_source)" check),
> or we need to arrange for 'program_source' to be initialized during
> guile's initialization before any other threads are created.

I'm happy to fix up the ones that I introduce.  There needs to be a
better abstraction for doing this lazy-init operation.  I might not get
around to making that abstraction before introducing a few more though;
apologies in advance for that.

They are actually very easy to search for, though.  Search for "  static
SCM" and you've got all of them :)

Andy
-- 
http://wingolog.org/



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-10-05 11:22 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <E1VRpou-0002qh-2M@vcs.savannah.gnu.org>
2013-10-04  4:20 ` [Guile-commits] GNU Guile branch, master, updated. v2.1.0-213-gb43e81d Mark H Weaver
2013-10-05 11:22   ` Andy Wingo

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).