unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* wip-threads-and-fork
@ 2012-02-08 22:10 Andy Wingo
  2012-02-22 21:40 ` wip-threads-and-fork Andy Wingo
  2012-03-01 19:32 ` wip-threads-and-fork Ludovic Courtès
  0 siblings, 2 replies; 32+ messages in thread
From: Andy Wingo @ 2012-02-08 22:10 UTC (permalink / raw)
  To: guile-devel; +Cc: Bruno Haible

Hi,

[Copying Bruno for an iconv question; see the end]

I was testing out the threaded web server and it was working well.  Then
I tried it out with tekuti, a blog engine that uses git as the backend.
It uses (ice-9 popen) to talk to the git binaries.  It was deadlocking
and segfaulting and all kinds of things!

The reason is that when you fork(), only the thread that calls fork()
survives into the child.  If another thread was in a critical section --
i.e., held a mutex -- it just stops.  The mutex remains taken, and
nothing will ever unlock it.  (Or is it in an undefined state?
Standards lawyers are welcome to input here.)  Of course whatever data
structures the threads are working on are in whatever inconsistent state
they were in as well.

This is a problem for Guile, even in the (ice-9 popen) case in which we
try to do the minimal amount of Schemely things before calling exec().
In particular there is the symbol table, which gets new things interned
due to make-prompt-tag (clearly prompt tags should not be symbols), and
there is the GC allocation lock, and there is the ports table, and the
mutexes on the individual ports (in master).

The solution is, besides just avoiding fork() and threads, to take locks
on all interesting data structures when you fork().  Fortunately there
are not too many, and most locks are not nested, so it seems to be a
doable thing.  In wip-threads-and-fork, I added a scm_c_atfork interface
to define functions to call before and after a fork.  It's like
pthread_atfork, though without the separate functions for parent and
child (is that needed?), and with the ability to have user data.  Also
the allocation lock is taken last.

I also added some CLOEXEC-related hacks to that branch.  We'll need a
new gnulib for accept4.

Finally, there was some mess with iconv().  iconv_open() is threadsafe,
but on glibc it loads gconv modules, within a lock.  It could be that
another thread was in iconv_open (and holding the gconv lock) when the
fork happens, preventing the child from doing scm_to_locale_string to
produce the argv for the execlp.  To fix this, I added locking around
all iconv usage, including indirect usage via libunistring.  This is
pretty nasty.  I guess the Right Thing would be a pthread_atfork()
within gconv; Bruno, is that correct?

I'm hesitant to do much threading-related work on stable-2.0, as master
has a much better story there.  Dunno.

Anyway, thoughts welcome.  With that patch series and
wip-threaded-web-server, I am able to happily hammer a test tekuti
instance without problems.  I'll try it out in production shortly.
Fingers crossed!

Cheers,

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-08 22:10 wip-threads-and-fork Andy Wingo
@ 2012-02-22 21:40 ` Andy Wingo
  2012-02-23 15:05   ` wip-threads-and-fork Andy Wingo
  2012-03-01 19:32 ` wip-threads-and-fork Ludovic Courtès
  1 sibling, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-02-22 21:40 UTC (permalink / raw)
  To: guile-devel; +Cc: Bruno Haible

Hi all,

An update on this issue.  For context:

On Wed 08 Feb 2012 23:10, Andy Wingo <wingo@pobox.com> writes:

> I was testing out the threaded web server and it was working well.  Then
> I tried it out with tekuti, a blog engine that uses git as the backend.
> It uses (ice-9 popen) to talk to the git binaries.  It was deadlocking
> and segfaulting and all kinds of things!
>
> The reason is that when you fork(), only the thread that calls fork()
> survives into the child.  If another thread was in a critical section --
> i.e., held a mutex -- it just stops.

One thing that I did not realize when I wrote this is that POSIX clearly
says that if a multithreaded program forks, the behavior of the child
after the fork is undefined if it calls any non-async-signal-safe
function before calling exec().

Go ahead and read that again, if you didn't catch it the first time.

Basically POSIX prohibits the generality of fork() if your program has
threads.  Guile has fork, and threads.  What to do?

Obviously we can treat the limited case of (ice-9 popen) in a more
portable fashion.

But then there is the question: why, in the name of all that is holy,
did POSIX specify pthread_atfork?  If its intention is to grab mutexes
around a fork, and mutex operations are not signal-safe, what are they
on about?

On Glibc, many things are likely to work.  Not all, of course -- witness
this bug:

> iconv_open() is threadsafe, but on glibc it loads gconv modules,
> within a lock.

Is the lack of a thread_atfork handler a glibc bug or not?  POSIX can
argue either way.

I filed http://sourceware.org/bugzilla/show_bug.cgi?id=13725 in any
case.  I'd rather not commit a workaround if it's a glibc bug, but what
about other platforms?  Do we in the Guile project have to choose
between threads and fork?

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-22 21:40 ` wip-threads-and-fork Andy Wingo
@ 2012-02-23 15:05   ` Andy Wingo
  2012-02-23 15:49     ` wip-threads-and-fork Nala Ginrut
  2012-02-26 22:00     ` wip-threads-and-fork Ludovic Courtès
  0 siblings, 2 replies; 32+ messages in thread
From: Andy Wingo @ 2012-02-23 15:05 UTC (permalink / raw)
  To: guile-devel; +Cc: Bruno Haible

On Wed 22 Feb 2012 22:40, Andy Wingo <wingo@pobox.com> writes:

> Obviously we can treat the limited case of (ice-9 popen) in a more
> portable fashion.

I have now rewritten open-process from ice-9 popen in C, so as to be
sure that only async-signal-safe routines get called.  This should make
ice-9 popen work reliably even in threaded environments.

> Do we in the Guile project have to choose between threads and fork?

I think the short answer here is simply "yes".  A guile built without
threads may fork to its heart's content.  However a guile built with
threads -- the default, recommended configuration -- should not call
primitive-fork.  Instead, it should use open-process.  I'll add
something to the docs.

In master, we should consider not providing primitive-fork, if Guile is
built with threads.  What do folks think about that?

Regards,

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-23 15:05   ` wip-threads-and-fork Andy Wingo
@ 2012-02-23 15:49     ` Nala Ginrut
  2012-02-23 16:13       ` wip-threads-and-fork Andy Wingo
  2012-02-26 22:00     ` wip-threads-and-fork Ludovic Courtès
  1 sibling, 1 reply; 32+ messages in thread
From: Nala Ginrut @ 2012-02-23 15:49 UTC (permalink / raw)
  To: Andy Wingo; +Cc: Bruno Haible, guile-devel

[-- Attachment #1: Type: text/plain, Size: 1321 bytes --]

On Thu, Feb 23, 2012 at 11:05 PM, Andy Wingo <wingo@pobox.com> wrote:

> On Wed 22 Feb 2012 22:40, Andy Wingo <wingo@pobox.com> writes:
>
> > Obviously we can treat the limited case of (ice-9 popen) in a more
> > portable fashion.
>
> I have now rewritten open-process from ice-9 popen in C, so as to be
> sure that only async-signal-safe routines get called.  This should make
> ice-9 popen work reliably even in threaded environments.
>
> > Do we in the Guile project have to choose between threads and fork?
>
> I think the short answer here is simply "yes".  A guile built without
> threads may fork to its heart's content.  However a guile built with
> threads -- the default, recommended configuration -- should not call
> primitive-fork.  Instead, it should use open-process.  I'll add
> something to the docs.
>
> In master, we should consider not providing primitive-fork, if Guile is
> built with threads.  What do folks think about that?
>
>
I just want to do my negative vote when I saw "choose thead then fork die",
but I see "open-process" soon. ;-)
 So, what's the difference between "primitive-fork" and "open-process"? If
they're different, I think much code to be modified for me...I believe I'm
not the only layman to use "fork" and "thread" both.


> Regards,
>
> Andy
> --
> http://wingolog.org/
>
>

[-- Attachment #2: Type: text/html, Size: 2223 bytes --]

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

* Re: wip-threads-and-fork
  2012-02-23 15:49     ` wip-threads-and-fork Nala Ginrut
@ 2012-02-23 16:13       ` Andy Wingo
  2012-02-24  3:00         ` wip-threads-and-fork Nala Ginrut
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-02-23 16:13 UTC (permalink / raw)
  To: Nala Ginrut; +Cc: Bruno Haible, guile-devel

On Thu 23 Feb 2012 16:49, Nala Ginrut <nalaginrut@gmail.com> writes:

> I just want to do my negative vote when I saw "choose thead then fork
> die", but I see "open-process" soon. ;-)  So, what's the difference
> between "primitive-fork" and "open-process"? If they're different, I
> think much code to be modified for me...I believe I'm not the only
> layman to use "fork" and "thread" both.  

Open-process is at the guts of the (ice-9 popen) modules.  Open-pipe* is
a thin wrapper around it -- see "Pipes" in the manual.

Basically open-pipe* / open-process does a fork(), and then an exec().
Doing this yourself in Scheme is actually impossible with threads,
because you'll need to take Scheme strings and produce C strings in the
current locale, to pass to the exec call.  That involves malloc, iconv,
and libgc allocation, and none of them are guaranteed to work after a
fork(), in a multithreaded program.

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-23 16:13       ` wip-threads-and-fork Andy Wingo
@ 2012-02-24  3:00         ` Nala Ginrut
  2012-02-24 10:21           ` wip-threads-and-fork Andy Wingo
  2012-02-24 18:57           ` wip-threads-and-fork Andy Wingo
  0 siblings, 2 replies; 32+ messages in thread
From: Nala Ginrut @ 2012-02-24  3:00 UTC (permalink / raw)
  To: Andy Wingo; +Cc: Bruno Haible, guile-devel

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

On Fri, Feb 24, 2012 at 12:13 AM, Andy Wingo <wingo@pobox.com> wrote:

> On Thu 23 Feb 2012 16:49, Nala Ginrut <nalaginrut@gmail.com> writes:
>
> > I just want to do my negative vote when I saw "choose thead then fork
> > die", but I see "open-process" soon. ;-)  So, what's the difference
> > between "primitive-fork" and "open-process"? If they're different, I
> > think much code to be modified for me...I believe I'm not the only
> > layman to use "fork" and "thread" both.
>
> Open-process is at the guts of the (ice-9 popen) modules.  Open-pipe* is
> a thin wrapper around it -- see "Pipes" in the manual.
>
> Basically open-pipe* / open-process does a fork(), and then an exec().
> Doing this yourself in Scheme is actually impossible with threads,
> because you'll need to take Scheme strings and produce C strings in the
> current locale, to pass to the exec call.  That involves malloc, iconv,
> and libgc allocation, and none of them are guaranteed to work after a
> fork(), in a multithreaded program.
>
>
I think I could use pipes to handle some sub-process rather than do it with
fork manually. But I must create a daemon, it can't avoid to use fork, will
this circumstance cause problems if I use threads after it?


> Andy
> --
> http://wingolog.org/
>

[-- Attachment #2: Type: text/html, Size: 2061 bytes --]

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

* Re: wip-threads-and-fork
  2012-02-24  3:00         ` wip-threads-and-fork Nala Ginrut
@ 2012-02-24 10:21           ` Andy Wingo
  2012-02-24 14:08             ` wip-threads-and-fork Nala Ginrut
  2012-02-26 22:03             ` wip-threads-and-fork Ludovic Courtès
  2012-02-24 18:57           ` wip-threads-and-fork Andy Wingo
  1 sibling, 2 replies; 32+ messages in thread
From: Andy Wingo @ 2012-02-24 10:21 UTC (permalink / raw)
  To: Nala Ginrut; +Cc: Bruno Haible, guile-devel

On Fri 24 Feb 2012 04:00, Nala Ginrut <nalaginrut@gmail.com> writes:

> I think I could use pipes to handle some sub-process rather than do it
> with fork manually. But I must create a daemon, it can't avoid to use
> fork, will this circumstance cause problems if I use threads after it?

I just pushed a patch like this:

    --- a/libguile/posix.c
    +++ b/libguile/posix.c
    @@ -1,4 +1,4 @@
    -/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
    +/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Free Software Foundation, Inc
      * 
      * This library is free software; you can redistribute it and/or
      * modify it under the terms of the GNU Lesser General Public License
    @@ -1248,6 +1248,18 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0,
     #define FUNC_NAME s_scm_fork
     {
       int pid;
    +  if (scm_ilength (scm_all_threads ()) != 1)
    +    /* Other threads may be holding on to resources that Guile needs --
    +       it is not safe to permit one thread to fork while others are
    +       running.
    +
    +       In addition, POSIX clearly specifies that if a multi-threaded
    +       program forks, the child must only call functions that are
    +       async-signal-safe.  We can't guarantee that in general.  The best
    +       we can do is to allow forking only very early, before any call to
    +       sigaction spawns the signal-handling thread.  */
    +    SCM_MISC_ERROR ("attempt to fork while multiple threads are running",
    +                    SCM_EOL);
       pid = fork ();
       if (pid == -1)
         SCM_SYSERROR;

What do you think?

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-24 10:21           ` wip-threads-and-fork Andy Wingo
@ 2012-02-24 14:08             ` Nala Ginrut
  2012-02-24 14:47               ` wip-threads-and-fork Andy Wingo
  2012-02-26 22:03             ` wip-threads-and-fork Ludovic Courtès
  1 sibling, 1 reply; 32+ messages in thread
From: Nala Ginrut @ 2012-02-24 14:08 UTC (permalink / raw)
  To: Andy Wingo; +Cc: Bruno Haible, guile-devel

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

On Fri, Feb 24, 2012 at 6:21 PM, Andy Wingo <wingo@pobox.com> wrote:

> On Fri 24 Feb 2012 04:00, Nala Ginrut <nalaginrut@gmail.com> writes:
>
> > I think I could use pipes to handle some sub-process rather than do it
> > with fork manually. But I must create a daemon, it can't avoid to use
> > fork, will this circumstance cause problems if I use threads after it?
>
> I just pushed a patch like this:
>
>    --- a/libguile/posix.c
>    +++ b/libguile/posix.c
>    @@ -1,4 +1,4 @@
>    -/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2003, 2004,
> 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
>    +/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2003, 2004,
> 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Free Software Foundation, Inc
>      *
>      * This library is free software; you can redistribute it and/or
>      * modify it under the terms of the GNU Lesser General Public License
>    @@ -1248,6 +1248,18 @@ SCM_DEFINE (scm_fork, "primitive-fork", 0, 0, 0,
>     #define FUNC_NAME s_scm_fork
>     {
>       int pid;
>    +  if (scm_ilength (scm_all_threads ()) != 1)
>    +    /* Other threads may be holding on to resources that Guile needs --
>    +       it is not safe to permit one thread to fork while others are
>    +       running.
>    +
>    +       In addition, POSIX clearly specifies that if a multi-threaded
>    +       program forks, the child must only call functions that are
>    +       async-signal-safe.  We can't guarantee that in general.  The
> best
>    +       we can do is to allow forking only very early, before any call
> to
>    +       sigaction spawns the signal-handling thread.  */
>    +    SCM_MISC_ERROR ("attempt to fork while multiple threads are
> running",
>    +                    SCM_EOL);
>

Well, at least it support to create a daemon.
Considering the new 'open-process' hasn't been done, the issues left
suspending.
And I'm glad to see the skills you deal with this problem soon. ;-)



>       pid = fork ();
>       if (pid == -1)
>         SCM_SYSERROR;
>
> What do you think?
>
> Andy
> --
> http://wingolog.org/
>

[-- Attachment #2: Type: text/html, Size: 3028 bytes --]

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

* Re: wip-threads-and-fork
  2012-02-24 14:08             ` wip-threads-and-fork Nala Ginrut
@ 2012-02-24 14:47               ` Andy Wingo
  2012-02-24 15:25                 ` wip-threads-and-fork Nala Ginrut
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-02-24 14:47 UTC (permalink / raw)
  To: Nala Ginrut; +Cc: Bruno Haible, guile-devel

Hi,

Thanks for the feedback :)

On Fri 24 Feb 2012 15:08, Nala Ginrut <nalaginrut@gmail.com> writes:

> Considering the new 'open-process' hasn't been done, the issues left
> suspending.

Maybe I have not expressed myself well.  To be clear, the new
open-process implementation is now in both master and 2.0, though not in
any release yet.  You can access it via open-pipe*.

Regards,

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-24 14:47               ` wip-threads-and-fork Andy Wingo
@ 2012-02-24 15:25                 ` Nala Ginrut
  0 siblings, 0 replies; 32+ messages in thread
From: Nala Ginrut @ 2012-02-24 15:25 UTC (permalink / raw)
  To: Andy Wingo; +Cc: Bruno Haible, guile-devel

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Fri, Feb 24, 2012 at 10:47 PM, Andy Wingo <wingo@pobox.com> wrote:

> Hi,
>
> Thanks for the feedback :)
>
> On Fri 24 Feb 2012 15:08, Nala Ginrut <nalaginrut@gmail.com> writes:
>
> > Considering the new 'open-process' hasn't been done, the issues left
> > suspending.
>
> Maybe I have not expressed myself well.  To be clear, the new
> open-process implementation is now in both master and 2.0, though not in
> any release yet.  You can access it via open-pipe*.
>
>
ah~I thought I've pulled repository today, but I didn't. Now I see it.
Thanks!


> Regards,
>
> Andy
> --
> http://wingolog.org/
>

[-- Attachment #2: Type: text/html, Size: 1286 bytes --]

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

* Re: wip-threads-and-fork
  2012-02-24  3:00         ` wip-threads-and-fork Nala Ginrut
  2012-02-24 10:21           ` wip-threads-and-fork Andy Wingo
@ 2012-02-24 18:57           ` Andy Wingo
  2012-02-25  2:21             ` wip-threads-and-fork Nala Ginrut
  1 sibling, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-02-24 18:57 UTC (permalink / raw)
  To: Nala Ginrut; +Cc: guile-devel

Hello :)

On Fri 24 Feb 2012 04:00, Nala Ginrut <nalaginrut@gmail.com> writes:

> I think I could use pipes to handle some sub-process rather than do it
> with fork manually. But I must create a daemon, it can't avoid to use
> fork, will this circumstance cause problems if I use threads after it?

Hummm.  Very good question.  I started to answer this, but ended up
committing the hack I mailed before, then found this reply now.

Basically, if you try to daemonize with fork, you would need to do it
very early, before your program makes any threads.  In master, Guile now
enforces that condition.

Note that the first time your program calls `sigaction', Guile spawns a
thread to handle signals.  You'd need to fork before installing signal
handlers.

Another thing to think about is libgc.  In the "master" branch, libgc
itself can spawn threads -- for example, a thread for doing parallel
marking.

There is a tension here between making things work, and making them
correct.  It is possible to get fork + threads mostly working on glibc
platforms -- for example, we could restart signal-handling and finalizer
threads in primitive-fork.  But it is not possible to make the use of
`primitive-fork' correct, in general, in the presence of threads.

Cheers,

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-24 18:57           ` wip-threads-and-fork Andy Wingo
@ 2012-02-25  2:21             ` Nala Ginrut
  2012-02-25  2:30               ` wip-threads-and-fork Nala Ginrut
  0 siblings, 1 reply; 32+ messages in thread
From: Nala Ginrut @ 2012-02-25  2:21 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

On Sat, Feb 25, 2012 at 2:57 AM, Andy Wingo <wingo@pobox.com> wrote:

> Hello :)
>
> On Fri 24 Feb 2012 04:00, Nala Ginrut <nalaginrut@gmail.com> writes:
>
> > I think I could use pipes to handle some sub-process rather than do it
> > with fork manually. But I must create a daemon, it can't avoid to use
> > fork, will this circumstance cause problems if I use threads after it?
>
> Hummm.  Very good question.  I started to answer this, but ended up
> committing the hack I mailed before, then found this reply now.
>
> Basically, if you try to daemonize with fork, you would need to do it
> very early, before your program makes any threads.  In master, Guile now
> enforces that condition.
>
>
So, this suggestion implies primitive-fork would be kept?


> Note that the first time your program calls `sigaction', Guile spawns a
> thread to handle signals.  You'd need to fork before installing signal
> handlers.
>
>
Fortunately. ;-)


> Another thing to think about is libgc.  In the "master" branch, libgc
> itself can spawn threads -- for example, a thread for doing parallel
> marking.
>
> There is a tension here between making things work, and making them
> correct.  It is possible to get fork + threads mostly working on glibc
> platforms -- for example, we could restart signal-handling and finalizer
> threads in primitive-fork.  But it is not possible to make the use of
> `primitive-fork' correct, in general, in the presence of threads.
>
>
I think all I need primitive-fork is just to daemonize, and I can try to do
it in the very early.
Moreover, primitive-fork would be restricted to be used for daemon to
anybody.
Considering this point, why not add a "daemonize" proc instead of
"primitive-fork"?


> Cheers,
>
> Andy
> --
> http://wingolog.org/
>

[-- Attachment #2: Type: text/html, Size: 2857 bytes --]

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

* Re: wip-threads-and-fork
  2012-02-25  2:21             ` wip-threads-and-fork Nala Ginrut
@ 2012-02-25  2:30               ` Nala Ginrut
  2012-02-25 18:01                 ` wip-threads-and-fork Andy Wingo
  0 siblings, 1 reply; 32+ messages in thread
From: Nala Ginrut @ 2012-02-25  2:30 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

On Sat, Feb 25, 2012 at 10:21 AM, Nala Ginrut <nalaginrut@gmail.com> wrote:

>
>
> On Sat, Feb 25, 2012 at 2:57 AM, Andy Wingo <wingo@pobox.com> wrote:
>
>> Hello :)
>>
>> On Fri 24 Feb 2012 04:00, Nala Ginrut <nalaginrut@gmail.com> writes:
>>
>> > I think I could use pipes to handle some sub-process rather than do it
>> > with fork manually. But I must create a daemon, it can't avoid to use
>> > fork, will this circumstance cause problems if I use threads after it?
>>
>> Hummm.  Very good question.  I started to answer this, but ended up
>> committing the hack I mailed before, then found this reply now.
>>
>> Basically, if you try to daemonize with fork, you would need to do it
>> very early, before your program makes any threads.  In master, Guile now
>> enforces that condition.
>>
>>
> So, this suggestion implies primitive-fork would be kept?
>
>
>> Note that the first time your program calls `sigaction', Guile spawns a
>> thread to handle signals.  You'd need to fork before installing signal
>> handlers.
>>
>>
> Fortunately. ;-)
>
>
>> Another thing to think about is libgc.  In the "master" branch, libgc
>> itself can spawn threads -- for example, a thread for doing parallel
>> marking.
>>
>> There is a tension here between making things work, and making them
>> correct.  It is possible to get fork + threads mostly working on glibc
>> platforms -- for example, we could restart signal-handling and finalizer
>> threads in primitive-fork.  But it is not possible to make the use of
>> `primitive-fork' correct, in general, in the presence of threads.
>>
>>
> I think all I need primitive-fork is just to daemonize, and I can try to
> do it in the very early.
> Moreover, primitive-fork would be restricted to be used for daemon to
> anybody.
> Considering this point, why not add a "daemonize" proc instead of
> "primitive-fork"?
>

Well, I missed something.
'primitive-fork' shouldn't be removed directly according to
previous discussion, unless the user chosen ptheads. If they do not use
threads, then primitive-fork would be useful, is that right?


> Cheers,
>>
>> Andy
>> --
>> http://wingolog.org/
>>
>
>

[-- Attachment #2: Type: text/html, Size: 3651 bytes --]

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

* Re: wip-threads-and-fork
  2012-02-25  2:30               ` wip-threads-and-fork Nala Ginrut
@ 2012-02-25 18:01                 ` Andy Wingo
  2012-02-26  2:35                   ` wip-threads-and-fork Nala Ginrut
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-02-25 18:01 UTC (permalink / raw)
  To: Nala Ginrut; +Cc: guile-devel

Hi!

On Sat 25 Feb 2012 03:30, Nala Ginrut <nalaginrut@gmail.com> writes:

> 'primitive-fork' shouldn't be removed directly according to
> previous discussion, unless the user chosen ptheads. If they do not
> use threads, then primitive-fork would be useful, is that right?  

Indeed.  In Guile master, primitive-fork is always there (if your system
supports fork).  However, invoking it will raise an error if you have
created other threads.  You can only use it before creating threads.

Is that good for your purposes?

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-25 18:01                 ` wip-threads-and-fork Andy Wingo
@ 2012-02-26  2:35                   ` Nala Ginrut
  0 siblings, 0 replies; 32+ messages in thread
From: Nala Ginrut @ 2012-02-26  2:35 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On Sun, Feb 26, 2012 at 2:01 AM, Andy Wingo <wingo@pobox.com> wrote:

> Hi!
>
> On Sat 25 Feb 2012 03:30, Nala Ginrut <nalaginrut@gmail.com> writes:
>
> > 'primitive-fork' shouldn't be removed directly according to
> > previous discussion, unless the user chosen ptheads. If they do not
> > use threads, then primitive-fork would be useful, is that right?
>
> Indeed.  In Guile master, primitive-fork is always there (if your system
> supports fork).  However, invoking it will raise an error if you have
> created other threads.  You can only use it before creating threads.
>
> Is that good for your purposes?
>
>
All right, I think my problem is done ;-)


> Andy
> --
> http://wingolog.org/
>

[-- Attachment #2: Type: text/html, Size: 1395 bytes --]

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

* Re: wip-threads-and-fork
  2012-02-23 15:05   ` wip-threads-and-fork Andy Wingo
  2012-02-23 15:49     ` wip-threads-and-fork Nala Ginrut
@ 2012-02-26 22:00     ` Ludovic Courtès
  2012-02-27  9:39       ` wip-threads-and-fork Andy Wingo
  1 sibling, 1 reply; 32+ messages in thread
From: Ludovic Courtès @ 2012-02-26 22:00 UTC (permalink / raw)
  To: guile-devel

Hi,

Andy Wingo <wingo@pobox.com> skribis:

> A guile built without
> threads may fork to its heart's content.  However a guile built with
> threads -- the default, recommended configuration -- should not call
> primitive-fork.

That’s a strong statement.  When the only threads are the main thread
and the signal thread, everything’s alright.  For example, this works
fine on GNU/Linux:

--8<---------------cut here---------------start------------->8---
(let ((p (primitive-fork)))
  (case p
    ((0)
     (sigaction SIGINT (lambda args
                         (format #t "kid ~a got ~a~%" (getpid) args)
                         (exit 0)))
     (let loop () (loop))
     (exit 1))
    (else
     (sleep 2)
     (kill p SIGINT)
     (format #t "killed ~a~%" p)
     (waitpid p))))
--8<---------------cut here---------------end--------------->8---

It works because the signal thread is stuck in a read(2) with no lock
taken.

Things that don’t work include this:

--8<---------------cut here---------------start------------->8---
(use-modules (ice-9 futures))

(let* ((f (future (begin (sleep 4) (getpid))))
       (p (primitive-fork)))
  (case p
    ((0)
     (format #t "kid -> ~a~%" (touch f)))
    (else
     (format #t "parent -> ~a~%" (touch f))
     (waitpid p))))
--8<---------------cut here---------------end--------------->8---

Here the child waits forever because it has only one thread.  As for
popen, that’s a bug (or undocumented limitation) of (ice-9 futures)
itself, more than anything else.

Ludo’.




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

* Re: wip-threads-and-fork
  2012-02-24 10:21           ` wip-threads-and-fork Andy Wingo
  2012-02-24 14:08             ` wip-threads-and-fork Nala Ginrut
@ 2012-02-26 22:03             ` Ludovic Courtès
  2012-02-27  9:44               ` wip-threads-and-fork Andy Wingo
  1 sibling, 1 reply; 32+ messages in thread
From: Ludovic Courtès @ 2012-02-26 22:03 UTC (permalink / raw)
  To: guile-devel

Hi,

Andy Wingo <wingo@pobox.com> skribis:

>     +  if (scm_ilength (scm_all_threads ()) != 1)
>     +    /* Other threads may be holding on to resources that Guile needs --
>     +       it is not safe to permit one thread to fork while others are
>     +       running.
>     +
>     +       In addition, POSIX clearly specifies that if a multi-threaded
>     +       program forks, the child must only call functions that are
>     +       async-signal-safe.  We can't guarantee that in general.  The best
>     +       we can do is to allow forking only very early, before any call to
>     +       sigaction spawns the signal-handling thread.  */
>     +    SCM_MISC_ERROR ("attempt to fork while multiple threads are running",
>     +                    SCM_EOL);

Just like fork(2) lets one shoot themself in the foot, I think this is
beyond libguile’s scope.  After all, libguile just wraps the OS
features, however crippled they may be.

Ludo’.




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

* Re: wip-threads-and-fork
  2012-02-26 22:00     ` wip-threads-and-fork Ludovic Courtès
@ 2012-02-27  9:39       ` Andy Wingo
  2012-03-01 19:35         ` wip-threads-and-fork Ludovic Courtès
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-02-27  9:39 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hi,

On Sun 26 Feb 2012 23:00, ludo@gnu.org (Ludovic Courtès) writes:

>> A guile built without
>> threads may fork to its heart's content.  However a guile built with
>> threads -- the default, recommended configuration -- should not call
>> primitive-fork.
>
> That’s a strong statement.

Don't shoot the messenger ;)  I tried hard to make it work, but there's
no getting around POSIX here.

> When the only threads are the main thread
> and the signal thread, everything’s alright.  For example, this works
> fine on GNU/Linux:
>
> (let ((p (primitive-fork)))
>   (case p
>     ((0)
>      (sigaction SIGINT (lambda args
>                          (format #t "kid ~a got ~a~%" (getpid) args)
>                          (exit 0)))
>      (let loop () (loop))
>      (exit 1))
>     (else
>      (sleep 2)
>      (kill p SIGINT)
>      (format #t "killed ~a~%" p)
>      (waitpid p))))
>
> It works because the signal thread is stuck in a read(2) with no lock
> taken.

"Works" ;-) It mostly works on GNU/Linux.  But not all the time!  Other
processes can send your thread signals -- indeed, one would expect that
it's for that reason that you installed a signal handler.  If the
signal-handling thread wakes up and queues an async, it could be in the
middle of allocation, hold the alloc lock, and thereby prevent the child
from allocating anything.  Or it could have the async mutex.

I really doubt that we can build a stable system on top of such shaky
guarantees.

I 

> Things that don’t work include this:
>
> (use-modules (ice-9 futures))
>
> (let* ((f (future (begin (sleep 4) (getpid))))
>        (p (primitive-fork)))
>   (case p
>     ((0)
>      (format #t "kid -> ~a~%" (touch f)))
>     (else
>      (format #t "parent -> ~a~%" (touch f))
>      (waitpid p))))
>
> Here the child waits forever because it has only one thread.

Or because some other thread has the allocation lock, or some other
lock, including even the gconv lock deep in libc, etc.

> As for popen, that’s a bug (or undocumented limitation) of (ice-9
> futures) itself, more than anything else.

Popen now works with threads.

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-26 22:03             ` wip-threads-and-fork Ludovic Courtès
@ 2012-02-27  9:44               ` Andy Wingo
  2012-03-01 19:40                 ` wip-threads-and-fork Ludovic Courtès
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-02-27  9:44 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

On Sun 26 Feb 2012 23:03, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> skribis:
>
>>     +  if (scm_ilength (scm_all_threads ()) != 1)
>>     +    /* Other threads may be holding on to resources that Guile needs --
>>     +       it is not safe to permit one thread to fork while others are
>>     +       running.
>>     +
>>     +       In addition, POSIX clearly specifies that if a multi-threaded
>>     +       program forks, the child must only call functions that are
>>     +       async-signal-safe.  We can't guarantee that in general.  The best
>>     +       we can do is to allow forking only very early, before any call to
>>     +       sigaction spawns the signal-handling thread.  */
>>     +    SCM_MISC_ERROR ("attempt to fork while multiple threads are running",
>>     +                    SCM_EOL);
>
> Just like fork(2) lets one shoot themself in the foot, I think this is
> beyond libguile’s scope.  After all, libguile just wraps the OS
> features, however crippled they may be.

Dunno.  That is certainly the case for things like close-fdes, and the
FFI.  But if someone really wants an unsafe fork, the FFI can give it to
them, right?  Keep in mind that portably speaking, you can't even call
`malloc' in the child, if you forked a program with multiple threads.

Primitive-fork is much less necessary now that open-process, the most
important use case, has been rewritten in C.

Finally, I think it's particularly important to constrain primitive-fork
because it prevents composition of various modules.  If I have a module
that uses futures, and a module that forks, I can't compose them.

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-02-08 22:10 wip-threads-and-fork Andy Wingo
  2012-02-22 21:40 ` wip-threads-and-fork Andy Wingo
@ 2012-03-01 19:32 ` Ludovic Courtès
  1 sibling, 0 replies; 32+ messages in thread
From: Ludovic Courtès @ 2012-03-01 19:32 UTC (permalink / raw)
  To: guile-devel

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

Hi!

Andy Wingo <wingo@pobox.com> skribis:

> The solution is, besides just avoiding fork() and threads, to take locks
> on all interesting data structures when you fork().  Fortunately there
> are not too many, and most locks are not nested, so it seems to be a
> doable thing.  In wip-threads-and-fork, I added a scm_c_atfork interface
> to define functions to call before and after a fork.

I finally looked in more details at the branch.  In particular, this
part looks great):

7329a5db * use scm_c_atfork_lock_static_mutex for guile's static mutexen
585eb4f7 * add scm_c_atfork_lock_static_mutex
9f6ac5d7 * add atfork interface

What about merging these in stable-2.0?  (With additional comments–e.g.,
to describe ‘scm_c_atfork’–and correct indentation, and without the
weak-set part of 9f6ac5d7.)

However, these patches don’t seem to handle the specific issue that
‘signal_delivery_thread’ could be holding the allocation lock when
‘fork’ runs.  What about doing something along these lines:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1734 bytes --]

diff --git a/libguile/scmsigs.c b/libguile/scmsigs.c
index 86fce0f..e062f24 100644
--- a/libguile/scmsigs.c
+++ b/libguile/scmsigs.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2004, 2006, 2007, 2008, 2009, 2011 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2004, 2006, 2007, 2008, 2009, 2011, 2012 Free Software Foundation, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
@@ -183,6 +183,12 @@ read_signal_pipe_data (void * data)
   return NULL;
 }
   
+/* Lock to protect async queuing.  Since `system-async-mark' conses, and
+   thus acquires the GC lock, this lock ensures that the signal thread
+   doesn't call `system-async-mark' in the middle of a `fork'.  */
+static scm_i_pthread_mutex_t signal_async_mutex =
+  SCM_I_PTHREAD_MUTEX_INITIALIZER;
+
 static SCM
 signal_delivery_thread (void *data)
 {
@@ -212,7 +218,11 @@ signal_delivery_thread (void *data)
 	  h = SCM_SIMPLE_VECTOR_REF (signal_handler_asyncs, sig);
 	  t = SCM_SIMPLE_VECTOR_REF (signal_handler_threads, sig);
 	  if (scm_is_true (h))
+	    {
+	      scm_i_pthread_mutex_lock (&signal_async_mutex);
 	    scm_system_async_mark_for_thread (h, t);
+	      scm_i_pthread_mutex_unlock (&signal_async_mutex);
+	    }
 	}
       else if (sigdata.n == 0)
 	break; /* the signal pipe was closed. */
@@ -713,6 +723,8 @@ scm_init_scmsigs ()
   signal_handler_asyncs = scm_c_make_vector (NSIG, SCM_BOOL_F);
   signal_handler_threads = scm_c_make_vector (NSIG, SCM_BOOL_F);
 
+  scm_c_atfork_lock_static_mutex (&signal_async_mutex);
+
   for (i = 0; i < NSIG; i++)
     {
 #ifdef HAVE_SIGACTION

[-- Attachment #3: Type: text/plain, Size: 18 bytes --]


Thanks,
Ludo’.

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

* Re: wip-threads-and-fork
  2012-02-27  9:39       ` wip-threads-and-fork Andy Wingo
@ 2012-03-01 19:35         ` Ludovic Courtès
  2012-03-03 16:32           ` wip-threads-and-fork Andy Wingo
  0 siblings, 1 reply; 32+ messages in thread
From: Ludovic Courtès @ 2012-03-01 19:35 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Hi!

Andy Wingo <wingo@pobox.com> skribis:

> On Sun 26 Feb 2012 23:00, ludo@gnu.org (Ludovic Courtès) writes:

[...]

>> When the only threads are the main thread
>> and the signal thread, everything’s alright.  For example, this works
>> fine on GNU/Linux:
>>
>> (let ((p (primitive-fork)))
>>   (case p
>>     ((0)
>>      (sigaction SIGINT (lambda args
>>                          (format #t "kid ~a got ~a~%" (getpid) args)
>>                          (exit 0)))
>>      (let loop () (loop))
>>      (exit 1))
>>     (else
>>      (sleep 2)
>>      (kill p SIGINT)
>>      (format #t "killed ~a~%" p)
>>      (waitpid p))))
>>
>> It works because the signal thread is stuck in a read(2) with no lock
>> taken.
>
> "Works" ;-) It mostly works on GNU/Linux.  But not all the time!  Other
> processes can send your thread signals -- indeed, one would expect that
> it's for that reason that you installed a signal handler.  If the
> signal-handling thread wakes up and queues an async, it could be in the
> middle of allocation, hold the alloc lock, and thereby prevent the child
> from allocating anything.  Or it could have the async mutex.

That’s highly unlikely, but yes, it could happen.

I still think that saying that “a guile built with threads […] should
not call primitive-fork” is too strong, because again, it’s very, very
unlikely to fail when (= 1 (length (all-threads))), GNU/Linux or not.

Let’s not scare our users more than necessary.  ;-)

And perhaps it can be fixed in 2.0 based on what ‘wip-threads-and-fork’
provides, as discussed in another message.

Thanks,
Ludo’.



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

* Re: wip-threads-and-fork
  2012-02-27  9:44               ` wip-threads-and-fork Andy Wingo
@ 2012-03-01 19:40                 ` Ludovic Courtès
  0 siblings, 0 replies; 32+ messages in thread
From: Ludovic Courtès @ 2012-03-01 19:40 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Hi,

Andy Wingo <wingo@pobox.com> skribis:

> On Sun 26 Feb 2012 23:03, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@pobox.com> skribis:
>>
>>>     +  if (scm_ilength (scm_all_threads ()) != 1)
>>>     +    /* Other threads may be holding on to resources that Guile needs --
>>>     +       it is not safe to permit one thread to fork while others are
>>>     +       running.
>>>     +
>>>     +       In addition, POSIX clearly specifies that if a multi-threaded
>>>     +       program forks, the child must only call functions that are
>>>     +       async-signal-safe.  We can't guarantee that in general.  The best
>>>     +       we can do is to allow forking only very early, before any call to
>>>     +       sigaction spawns the signal-handling thread.  */
>>>     +    SCM_MISC_ERROR ("attempt to fork while multiple threads are running",
>>>     +                    SCM_EOL);
>>
>> Just like fork(2) lets one shoot themself in the foot, I think this is
>> beyond libguile’s scope.  After all, libguile just wraps the OS
>> features, however crippled they may be.
>
> Dunno.  That is certainly the case for things like close-fdes, and the
> FFI.  But if someone really wants an unsafe fork, the FFI can give it to
> them, right?  Keep in mind that portably speaking, you can't even call
> `malloc' in the child, if you forked a program with multiple threads.

How so?

> Primitive-fork is much less necessary now that open-process, the most
> important use case, has been rewritten in C.

That’s right.

Another option would be to expose ‘pthread_atfork’ to Scheme.  It would
allow, for instance, (ice-9 futures) to register a child-side handler to
create a new thread pool, which would sound useful to me.

WDYT?

Thanks,
Ludo’.



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

* Re: wip-threads-and-fork
  2012-03-01 19:35         ` wip-threads-and-fork Ludovic Courtès
@ 2012-03-03 16:32           ` Andy Wingo
  2012-03-03 21:20             ` wip-threads-and-fork Ludovic Courtès
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-03-03 16:32 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hello :)

On Thu 01 Mar 2012 20:35, ludo@gnu.org (Ludovic Courtès) writes:

> I still think that saying that “a guile built with threads […] should
> not call primitive-fork” is too strong, because again, it’s very, very
> unlikely to fail when (= 1 (length (all-threads))), GNU/Linux or not.

OK, agreed.  In fact it *should* work if there are no other threads.

How do you feel about raising an error if primitive-fork is called, and
there are other threads running?

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-03-03 16:32           ` wip-threads-and-fork Andy Wingo
@ 2012-03-03 21:20             ` Ludovic Courtès
  2012-03-04 11:38               ` wip-threads-and-fork Andy Wingo
  0 siblings, 1 reply; 32+ messages in thread
From: Ludovic Courtès @ 2012-03-03 21:20 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Hey!

Andy Wingo <wingo@pobox.com> skribis:

> How do you feel about raising an error if primitive-fork is called, and
> there are other threads running?

I think I’d prefer a solution where libguile-internal threads and locks
are suitably handled upon fork (basically what wip-threads-and-fork
does), and where users are provided with mechanisms to do the same at
their level–which boils down to exposing pthread_atfork.

WDYT?

Thanks,
Ludo’.



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

* Re: wip-threads-and-fork
  2012-03-03 21:20             ` wip-threads-and-fork Ludovic Courtès
@ 2012-03-04 11:38               ` Andy Wingo
  2012-03-21 21:26                 ` wip-threads-and-fork Ludovic Courtès
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-03-04 11:38 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

On Sat 03 Mar 2012 22:20, ludo@gnu.org (Ludovic Courtès) writes:

> I’d prefer a solution where libguile-internal threads and locks are
> suitably handled upon fork (basically what wip-threads-and-fork does),
> and where users are provided with mechanisms to do the same at their
> level–which boils down to exposing pthread_atfork.
>
> WDYT?

I would have preferred this, but I came to the conclusion that this
approach is not sound.

Did you see that I merged the atfork bits into master?
(wip-threads-and-fork also had some CLOEXEC bits that needed more
baking).  They worked... sorta.  They had a few problems:

  1) It's impossible to work around the lack of atfork() in libraries
     that you depend on.

     For example, wip-threads-and-fork called fork() within the GC alloc
     lock, to get around the lack of a pthread_atfork() in libgc.  But
     then I submitted a patch to make libgc do this itself:

       http://thread.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/4940

     It's pretty difficult to tell which version of libgc you would
     have.  There is no workaround that is sufficient.

  2) POSIX explicitly disclaims the result of calling non-signal-safe
     primitives after a fork() of a multithreaded program.

  3) Nobody cares about these bugs.  See e.g. the lack of response at
     http://sourceware.org/bugzilla/show_bug.cgi?id=13725.  Even Bruno
     didn't reply to the Cc.  See point (2).

  4) The atfork mechanism imposes a total ordering on locks.  This is
     possible for static locks, but difficult for locks on collectable
     Scheme objects.

  5) Relatedly, just to be able to lock all weak tables at a fork, we
     had to create a new weak table-of-tables and add the tables to it.
     This is needless complication and overhead.

  6) scm_c_atfork() is a broken interface.  Because it hangs its hooks
     off of one pthread_atfork() invocation, it can cause newer locks to
     insert themselves in the wrong position relative to
     pthread_atfork() calls made between when Guile installed the
     scm_c_atfork handler, and the call to scm_c_atfork.

     There can be only one pthread_atfork() list, in a correct program.

In the end I reverted those patches because they were just complication
that didn't solve any fundamental problems.

I came to the opinion, having run a threaded, forking program, that we
would be much better off if we provided good abstractions to spawn
processes, but that expecting fork() to work in a multithreaded program
is not realistic.

Still, there is one other thing that perhaps we could do to shut down
the signal handling thread around a fork().  Dunno, perhaps it is worth
looking into.

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-03-04 11:38               ` wip-threads-and-fork Andy Wingo
@ 2012-03-21 21:26                 ` Ludovic Courtès
  2012-03-22  2:48                   ` wip-threads-and-fork Nala Ginrut
  2012-03-23  9:40                   ` wip-threads-and-fork Andy Wingo
  0 siblings, 2 replies; 32+ messages in thread
From: Ludovic Courtès @ 2012-03-21 21:26 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Hello!

(With delay...)

Andy Wingo <wingo@pobox.com> skribis:

> I would have preferred this, but I came to the conclusion that this
> approach is not sound.

Without exposing ‘pthread_atfork’, how would you suggest making user
code “fork-safe”?  A use case would be reviving the futures thread pool
after ‘fork’.

> Did you see that I merged the atfork bits into master?
> (wip-threads-and-fork also had some CLOEXEC bits that needed more
> baking).  They worked... sorta.  They had a few problems:
>
>   1) It's impossible to work around the lack of atfork() in libraries
>      that you depend on.
>
>      For example, wip-threads-and-fork called fork() within the GC alloc
>      lock, to get around the lack of a pthread_atfork() in libgc.  But
>      then I submitted a patch to make libgc do this itself:
>
>        http://thread.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/4940
>
>      It's pretty difficult to tell which version of libgc you would
>      have.  There is no workaround that is sufficient.

Indeed, good point.

>   2) POSIX explicitly disclaims the result of calling non-signal-safe
>      primitives after a fork() of a multithreaded program.

Right, though reality seems to be more pleasant than POSIX. ;-)

>   3) Nobody cares about these bugs.  See e.g. the lack of response at
>      http://sourceware.org/bugzilla/show_bug.cgi?id=13725.  Even Bruno
>      didn't reply to the Cc.  See point (2).
>
>   4) The atfork mechanism imposes a total ordering on locks.  This is
>      possible for static locks, but difficult for locks on collectable
>      Scheme objects.
>
>   5) Relatedly, just to be able to lock all weak tables at a fork, we
>      had to create a new weak table-of-tables and add the tables to it.
>      This is needless complication and overhead.
>
>   6) scm_c_atfork() is a broken interface.  Because it hangs its hooks
>      off of one pthread_atfork() invocation, it can cause newer locks to
>      insert themselves in the wrong position relative to
>      pthread_atfork() calls made between when Guile installed the
>      scm_c_atfork handler, and the call to scm_c_atfork.
>
>      There can be only one pthread_atfork() list, in a correct program.

OK, thanks for the nice summary.  Indeed, this is a complex story.

> In the end I reverted those patches because they were just complication
> that didn't solve any fundamental problems.

OK.

> I came to the opinion, having run a threaded, forking program, that we
> would be much better off if we provided good abstractions to spawn
> processes, but that expecting fork() to work in a multithreaded program
> is not realistic.

Yes, things like ‘open-process’ make sense.

What about adding a big fat warning in the doc about use of
‘primitive-fork’ in a multi-threaded program?

> Still, there is one other thing that perhaps we could do to shut down
> the signal handling thread around a fork().  Dunno, perhaps it is worth
> looking into.

What would be the expected benefit?

Thanks,
Ludo’.



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

* Re: wip-threads-and-fork
  2012-03-21 21:26                 ` wip-threads-and-fork Ludovic Courtès
@ 2012-03-22  2:48                   ` Nala Ginrut
  2012-03-23  9:40                   ` wip-threads-and-fork Andy Wingo
  1 sibling, 0 replies; 32+ messages in thread
From: Nala Ginrut @ 2012-03-22  2:48 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: Andy Wingo, guile-devel

[-- Attachment #1: Type: text/plain, Size: 3386 bytes --]

On Thu, Mar 22, 2012 at 5:26 AM, Ludovic Courtès <ludo@gnu.org> wrote:

> Hello!
>
> (With delay...)
>
> Andy Wingo <wingo@pobox.com> skribis:
>
> > I would have preferred this, but I came to the conclusion that this
> > approach is not sound.
>
> Without exposing ‘pthread_atfork’, how would you suggest making user
> code “fork-safe”?  A use case would be reviving the futures thread pool
> after ‘fork’.
>
> > Did you see that I merged the atfork bits into master?
> > (wip-threads-and-fork also had some CLOEXEC bits that needed more
> > baking).  They worked... sorta.  They had a few problems:
> >
> >   1) It's impossible to work around the lack of atfork() in libraries
> >      that you depend on.
> >
> >      For example, wip-threads-and-fork called fork() within the GC alloc
> >      lock, to get around the lack of a pthread_atfork() in libgc.  But
> >      then I submitted a patch to make libgc do this itself:
> >
> >
> http://thread.gmane.org/gmane.comp.programming.garbage-collection.boehmgc/4940
> >
> >      It's pretty difficult to tell which version of libgc you would
> >      have.  There is no workaround that is sufficient.
>
> Indeed, good point.
>
> >   2) POSIX explicitly disclaims the result of calling non-signal-safe
> >      primitives after a fork() of a multithreaded program.
>
> Right, though reality seems to be more pleasant than POSIX. ;-)
>
> >   3) Nobody cares about these bugs.  See e.g. the lack of response at
> >      http://sourceware.org/bugzilla/show_bug.cgi?id=13725.  Even Bruno
> >      didn't reply to the Cc.  See point (2).
> >
> >   4) The atfork mechanism imposes a total ordering on locks.  This is
> >      possible for static locks, but difficult for locks on collectable
> >      Scheme objects.
> >
> >   5) Relatedly, just to be able to lock all weak tables at a fork, we
> >      had to create a new weak table-of-tables and add the tables to it.
> >      This is needless complication and overhead.
> >
> >   6) scm_c_atfork() is a broken interface.  Because it hangs its hooks
> >      off of one pthread_atfork() invocation, it can cause newer locks to
> >      insert themselves in the wrong position relative to
> >      pthread_atfork() calls made between when Guile installed the
> >      scm_c_atfork handler, and the call to scm_c_atfork.
> >
> >      There can be only one pthread_atfork() list, in a correct program.
>
> OK, thanks for the nice summary.  Indeed, this is a complex story.
>
> > In the end I reverted those patches because they were just complication
> > that didn't solve any fundamental problems.
>
> OK.
>
> > I came to the opinion, having run a threaded, forking program, that we
> > would be much better off if we provided good abstractions to spawn
> > processes, but that expecting fork() to work in a multithreaded program
> > is not realistic.
>
> Yes, things like ‘open-process’ make sense.
>
> What about adding a big fat warning in the doc about use of
> ‘primitive-fork’ in a multi-threaded program?
>

Please do it! My 5 cents.


>
> > Still, there is one other thing that perhaps we could do to shut down
> > the signal handling thread around a fork().  Dunno, perhaps it is worth
> > looking into.
>
> What would be the expected benefit?
>
> Thanks,
> Ludo’.
>
>

[-- Attachment #2: Type: text/html, Size: 4563 bytes --]

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

* Re: wip-threads-and-fork
  2012-03-21 21:26                 ` wip-threads-and-fork Ludovic Courtès
  2012-03-22  2:48                   ` wip-threads-and-fork Nala Ginrut
@ 2012-03-23  9:40                   ` Andy Wingo
  2012-03-27 15:54                     ` wip-threads-and-fork Ludovic Courtès
  1 sibling, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-03-23  9:40 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hi :)

On Wed 21 Mar 2012 22:26, ludo@gnu.org (Ludovic Courtès) writes:

> Without exposing ‘pthread_atfork’, how would you suggest making user
> code “fork-safe”?  A use case would be reviving the futures thread pool
> after ‘fork’.

Humm, just tried to think about it for 10 minutes or so and I'm not
coming up with anything.  I don't think it's possible...

>> I came to the opinion, having run a threaded, forking program, that we
>> would be much better off if we provided good abstractions to spawn
>> processes, but that expecting fork() to work in a multithreaded program
>> is not realistic.
>
> Yes, things like ‘open-process’ make sense.
>
> What about adding a big fat warning in the doc about use of
> ‘primitive-fork’ in a multi-threaded program?

Sure.  WDYT about a warning at runtime as well?  (I assume you still
don't like the current master behavior of throwing an exception if other
threads are running?)

>> Still, there is one other thing that perhaps we could do to shut down
>> the signal handling thread around a fork().  Dunno, perhaps it is worth
>> looking into.
>
> What would be the expected benefit?

Let's assume a knowledgable user writing a program.  The user goes to
call primitive-fork, and thinks, "Am I using threads in this program?
Because if I am, it's not safe for me to fork."  The user looks and sees
that up to that point, no threads are used, so the user goes ahead.

If Guile started any threads on the user's behalf, this would invalidate
this user's mental calculation.

Currently in master, we stop the finalizer thread from within
primitive-fork, leaving the next finalizer run to spawn it again.

We could do something similar with the signal thread.

The alternative is to document "don't fork after calling sigaction".

Regards,

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-03-23  9:40                   ` wip-threads-and-fork Andy Wingo
@ 2012-03-27 15:54                     ` Ludovic Courtès
  2012-04-06 18:30                       ` wip-threads-and-fork Andy Wingo
  0 siblings, 1 reply; 32+ messages in thread
From: Ludovic Courtès @ 2012-03-27 15:54 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Hi,

Andy Wingo <wingo@pobox.com> skribis:

> On Wed 21 Mar 2012 22:26, ludo@gnu.org (Ludovic Courtès) writes:

[...]

>> Yes, things like ‘open-process’ make sense.
>>
>> What about adding a big fat warning in the doc about use of
>> ‘primitive-fork’ in a multi-threaded program?
>
> Sure.  WDYT about a warning at runtime as well?  (I assume you still
> don't like the current master behavior of throwing an exception if other
> threads are running?)

I agree that ‘primitive-fork’ has a number of important shortcomings.
I’m not convinced that anything needs to be done in the code itself,
though.  It’s just as harmful as ‘pointer->procedure’, in a way.

>>> Still, there is one other thing that perhaps we could do to shut down
>>> the signal handling thread around a fork().  Dunno, perhaps it is worth
>>> looking into.
>>
>> What would be the expected benefit?
>
> Let's assume a knowledgable user writing a program.  The user goes to
> call primitive-fork, and thinks, "Am I using threads in this program?
> Because if I am, it's not safe for me to fork."  The user looks and sees
> that up to that point, no threads are used, so the user goes ahead.
>
> If Guile started any threads on the user's behalf, this would invalidate
> this user's mental calculation.

Yes, so internal threads could be handled specially.  It seems the
signal thread already gets restarted magically, for instance, thanks to
the various scm_i_ensure_signal_delivery_thread calls.

Thanks,
Ludo’.



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

* Re: wip-threads-and-fork
  2012-03-27 15:54                     ` wip-threads-and-fork Ludovic Courtès
@ 2012-04-06 18:30                       ` Andy Wingo
  2012-04-07 22:54                         ` wip-threads-and-fork Ludovic Courtès
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Wingo @ 2012-04-06 18:30 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Heya :)

An ongoing discussion...

On Tue 27 Mar 2012 08:54, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> skribis:
>
>> On Wed 21 Mar 2012 22:26, ludo@gnu.org (Ludovic Courtès) writes:
>>
>>> What about adding a big fat warning in the doc about use of
>>> ‘primitive-fork’ in a multi-threaded program?
>>
>> Sure.  WDYT about a warning at runtime as well?  (I assume you still
>> don't like the current master behavior of throwing an exception if other
>> threads are running?)
>
> I agree that ‘primitive-fork’ has a number of important shortcomings.
> I’m not convinced that anything needs to be done in the code itself,
> though.  It’s just as harmful as ‘pointer->procedure’, in a way.

Indeed.  One more point though, not to totally drive this discussion
into the ground: primitive-fork is unlike pointer->procedure in that its
correctness depends on global conditions rather than local conditions.

If I load up a library and dlsym an address, and I believe that the
library has some kind of interface stability, then using
pointer->procedure is correct -- independent of what other Guile modules
are doing.

Whereas, the correctness of using primitive-fork in a module depends on
what other modules do.  Even if your module that uses primitive-fork
doesn't interact with anything else, it suddenly becomes incorrect if
the user loads up some threads-using library, like an asynchronous DNS
resolver implemented using getaddrinfo and thread-pools.

If other threads are active, I think primitive-fork should raise an
error.  But if you think warnings are the way to go, I'm OK with that
too.  Do you still think warnings are the right answer?

> Yes, so internal threads could be handled specially.  It seems the
> signal thread already gets restarted magically, for instance, thanks to
> the various scm_i_ensure_signal_delivery_thread calls.

Cool, we should add some tests that this works.  I think it needs a
little implementation help currently.  I'll take a look.

Cheers,

Andy
-- 
http://wingolog.org/



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

* Re: wip-threads-and-fork
  2012-04-06 18:30                       ` wip-threads-and-fork Andy Wingo
@ 2012-04-07 22:54                         ` Ludovic Courtès
  2013-01-17 11:41                           ` wip-threads-and-fork Andy Wingo
  0 siblings, 1 reply; 32+ messages in thread
From: Ludovic Courtès @ 2012-04-07 22:54 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guile-devel

Hey!

Andy Wingo <wingo@pobox.com> skribis:

> On Tue 27 Mar 2012 08:54, ludo@gnu.org (Ludovic Courtès) writes:
>
>> Andy Wingo <wingo@pobox.com> skribis:
>>
>>> On Wed 21 Mar 2012 22:26, ludo@gnu.org (Ludovic Courtès) writes:
>>>
>>>> What about adding a big fat warning in the doc about use of
>>>> ‘primitive-fork’ in a multi-threaded program?
>>>
>>> Sure.  WDYT about a warning at runtime as well?  (I assume you still
>>> don't like the current master behavior of throwing an exception if other
>>> threads are running?)
>>
>> I agree that ‘primitive-fork’ has a number of important shortcomings.
>> I’m not convinced that anything needs to be done in the code itself,
>> though.  It’s just as harmful as ‘pointer->procedure’, in a way.
>
> Indeed.  One more point though, not to totally drive this discussion
> into the ground: primitive-fork is unlike pointer->procedure in that its
> correctness depends on global conditions rather than local conditions.
>
> If I load up a library and dlsym an address, and I believe that the
> library has some kind of interface stability, then using
> pointer->procedure is correct -- independent of what other Guile modules
> are doing.
>
> Whereas, the correctness of using primitive-fork in a module depends on
> what other modules do.  Even if your module that uses primitive-fork
> doesn't interact with anything else, it suddenly becomes incorrect if
> the user loads up some threads-using library, like an asynchronous DNS
> resolver implemented using getaddrinfo and thread-pools.

Right.

> If other threads are active, I think primitive-fork should raise an
> error.  But if you think warnings are the way to go, I'm OK with that
> too.  Do you still think warnings are the right answer?

I think so, and with a caveat in the manual.

Thanks for your patience.  :-)

Ludo’.



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

* Re: wip-threads-and-fork
  2012-04-07 22:54                         ` wip-threads-and-fork Ludovic Courtès
@ 2013-01-17 11:41                           ` Andy Wingo
  0 siblings, 0 replies; 32+ messages in thread
From: Andy Wingo @ 2013-01-17 11:41 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

Hi,

>>>> On Wed 21 Mar 2012 22:26, ludo@gnu.org (Ludovic Courtès) writes:
>>>>
>>>>> What about adding a big fat warning in the doc about use of
>>>>> ‘primitive-fork’ in a multi-threaded program?

> Andy Wingo <wingo@pobox.com> skribis:
>
>> If other threads are active, I think primitive-fork should raise an
>> error.  But if you think warnings are the way to go, I'm OK with that
>> too.  Do you still think warnings are the right answer?
>
> I think so, and with a caveat in the manual.

Done: added a warning to stable-2.0, a note to the docs, and changed the
error to a warning in master.

Andy
-- 
http://wingolog.org/



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

end of thread, other threads:[~2013-01-17 11:41 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-08 22:10 wip-threads-and-fork Andy Wingo
2012-02-22 21:40 ` wip-threads-and-fork Andy Wingo
2012-02-23 15:05   ` wip-threads-and-fork Andy Wingo
2012-02-23 15:49     ` wip-threads-and-fork Nala Ginrut
2012-02-23 16:13       ` wip-threads-and-fork Andy Wingo
2012-02-24  3:00         ` wip-threads-and-fork Nala Ginrut
2012-02-24 10:21           ` wip-threads-and-fork Andy Wingo
2012-02-24 14:08             ` wip-threads-and-fork Nala Ginrut
2012-02-24 14:47               ` wip-threads-and-fork Andy Wingo
2012-02-24 15:25                 ` wip-threads-and-fork Nala Ginrut
2012-02-26 22:03             ` wip-threads-and-fork Ludovic Courtès
2012-02-27  9:44               ` wip-threads-and-fork Andy Wingo
2012-03-01 19:40                 ` wip-threads-and-fork Ludovic Courtès
2012-02-24 18:57           ` wip-threads-and-fork Andy Wingo
2012-02-25  2:21             ` wip-threads-and-fork Nala Ginrut
2012-02-25  2:30               ` wip-threads-and-fork Nala Ginrut
2012-02-25 18:01                 ` wip-threads-and-fork Andy Wingo
2012-02-26  2:35                   ` wip-threads-and-fork Nala Ginrut
2012-02-26 22:00     ` wip-threads-and-fork Ludovic Courtès
2012-02-27  9:39       ` wip-threads-and-fork Andy Wingo
2012-03-01 19:35         ` wip-threads-and-fork Ludovic Courtès
2012-03-03 16:32           ` wip-threads-and-fork Andy Wingo
2012-03-03 21:20             ` wip-threads-and-fork Ludovic Courtès
2012-03-04 11:38               ` wip-threads-and-fork Andy Wingo
2012-03-21 21:26                 ` wip-threads-and-fork Ludovic Courtès
2012-03-22  2:48                   ` wip-threads-and-fork Nala Ginrut
2012-03-23  9:40                   ` wip-threads-and-fork Andy Wingo
2012-03-27 15:54                     ` wip-threads-and-fork Ludovic Courtès
2012-04-06 18:30                       ` wip-threads-and-fork Andy Wingo
2012-04-07 22:54                         ` wip-threads-and-fork Ludovic Courtès
2013-01-17 11:41                           ` wip-threads-and-fork Andy Wingo
2012-03-01 19:32 ` wip-threads-and-fork Ludovic Courtès

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