debugging guile runtime

debugging guile runtime

[rixed]

Hello !

I'm still annoyed by the runtime bugs related to ports/threads at work,
so I can devote some time to work on it.

So far, the pending problems are, in order of importance for me :

#1 ports are not thread safe (and any other thread safety issues) ;
#2 fork may freeze in some occurrence ;
#3 the use of select prevent the extended app to open more than 1024
   files ;
#4 fork does not close all open files.
#5 new syntax definitions are not loaded by compiler

for #3, I already have a patch (that I sent here some months ago but was
neither rejected nor accepted as far as I know), for #5 there are
various easy workarounds, but for other bugs I am still stuck.  So, I'm
willing to work on those if someone who's already familiar with the
runtime can give some guidance.

Re: debugging guile runtime

[Nala Ginrut]

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

On Mon, Aug 29, 2011 at 8:50 PM, <rixed@happyleptic.org> wrote:

#4 fork does not close all open files.
>

well, I got the same question, will fork auto close in Guile or should I
close it myself?

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

Re: debugging guile runtime

[rixed]

-[ Mon, Aug 29, 2011 at 10:35:20PM +0800, Nala Ginrut ]----
> On Mon, Aug 29, 2011 at 8:50 PM, <rixed@happyleptic.org> wrote:
> 
> #4 fork does not close all open files.
> >
> 
> well, I got the same question, will fork auto close in Guile or should I
> close it myself?

Sorry I did not report the problem very well.  The trouble is not with
fork itself but with open-process, which is not actually part of the
runtime but rather part of ice-9 popen module.

Re: debugging guile runtime

[Thien-Thi Nguyen]

() rixed@happyleptic.org
() Mon, 29 Aug 2011 18:24:00 +0200

   Sorry I did not report the problem very well.  The trouble is not with
   fork itself but with open-process, which is not actually part of the
   runtime but rather part of ice-9 popen module.

Perhaps an object property could be added:

;; The property that @code{execl} uses to decide how to handle
;; an open port.  If the value is @code{#t}, do @code{close-port}
;; on it.  If a procedure, call that procedure with the port.
;; If @code{#f} or never specified, do nothing.
;;
(define close-on-exec (make-object-property))

Example usage:

(define port (open-input-file "COPYING"))
(set! (close-on-exec port) #t)

Re: debugging guile runtime

[rixed]

The problem is not that the *ports* are not closed (they are), but that
the other *files* (that are not port) are not closed.

When a program exec another one, it's supposed to close all files but 0,
1 and 2. open-process only closes the ports, so if you have other files
opened (that you have opened from C for instance) these files will be
communicated to the new process, which is not the intended behavior.

I state that open-process should _not_ try to close *ports*, but should
close all *files* (but 0, 1 and 2) instead.

Re: debugging guile runtime

[Ludovic Courtès]

Hi!

rixed@happyleptic.org skribis:

> The problem is not that the *ports* are not closed (they are), but that
> the other *files* (that are not port) are not closed.

Like in C, it’s up to the application to close those ports that it
considers worth closing upon exec.

If what you want is to close every FD from 3 to ∞, then the typical way
to do this is to loop over those numbers and call close(2) (that’s how
libdaemon and similar tools do that.)

Or did I miss something?

Thanks,
Ludo’.

Re: debugging guile runtime

[rixed]

-[ Tue, Aug 30, 2011 at 05:55:22PM +0200, Ludovic Courtès ]----
> Like in C, it???s up to the application to close those ports that it
> considers worth closing upon exec.

I was under the impression that the idiom was to close all files before
execing a coprocess, but I just checked POSIX popen, R.Stevens and libslack
and none does that. Well, the lib I'm used to does that and it felt
natural to do so that I erroneously assumed it was a mandated behavior
(so that running a coprocess is similar to running a program from a
shell).

OK then, so it's not a bug and I have to live with this behavior
(BTW, if anyone knows the rational behind this, I would be glad to
know).

> If what you want is to close every FD from 3 to ???, then the typical way
> to do this is to loop over those numbers and call close(2) (that???s how
> libdaemon and similar tools do that.)

Except that, as previously discussed in another thread, there is no easy
way to do that between the fork and the exec (since I don't want to
actually close these files in the main program that starts the pipe).

Anyway, what about the other bugs?
Is there anything I could do to advance the thread-safety issue for instance?

Re: debugging guile runtime

[Ludovic Courtès]

Hi,

rixed@happyleptic.org skribis:

> -[ Tue, Aug 30, 2011 at 05:55:22PM +0200, Ludovic Courtès ]----
>> Like in C, it???s up to the application to close those ports that it
>> considers worth closing upon exec.

[...]

> OK then, so it's not a bug and I have to live with this behavior
> (BTW, if anyone knows the rational behind this, I would be glad to
> know).

When typing “foo 2< baz 5> bar” in a shell, the shell first forks, the
opens/dups 2 and 5, and finally execs ‘foo’, which gets to see 2 and 5
as open FDs.  Same for pipes.

> Except that, as previously discussed in another thread, there is no easy
> way to do that between the fork and the exec (since I don't want to
> actually close these files in the main program that starts the pipe).

Hmm right.  We’d need to insert a hook in ‘open-pipe’ so that, when this
behavior is wanted, one can actually close those FDs.

Ideas?

Thanks,
Ludo’.

Re: debugging guile runtime

[rixed]

> > (BTW, if anyone knows the rational behind this, I would be glad to
> > know).
> 
> When typing ???foo 2< baz 5> bar??? in a shell, the shell first forks, the
> opens/dups 2 and 5, and finally execs ???foo???, which gets to see 2 and 5
> as open FDs.

Yes, the shell can open/dup the file additional files you explicitly
ask for, but the point is that it will nonetheless closes the file
descriptors it is using itself.  For instance my shell is using these
files :

$ ls -l /proc/$(pidof zsh)/fd
lrwx------ 1 rixed rixed 64 Aug 31 09:25 0 -> /dev/tty1
lrwx------ 1 rixed rixed 64 Aug 31 09:25 1 -> /dev/tty1
lrwx------ 1 rixed rixed 64 Aug 31 09:25 10 -> /dev/tty1
lr-x------ 1 rixed rixed 64 Aug 31 09:25 11 -> /debian/home/rixed/.zshenv
lr-x------ 1 rixed rixed 64 Aug 31 09:25 12 -> /debian/home/rixed/.profile
lrwx------ 1 rixed rixed 64 Aug 31 09:25 2 -> /dev/tty1

but the children it execs do not come with files 10, 11 and 12 opened.
So this is my understanding that when the shell exec a command it closes
all but the required files between fork and exec, while popen do not.

This is getting off-topic, though.

> > Except that, as previously discussed in another thread, there is no easy
> > way to do that between the fork and the exec (since I don't want to
> > actually close these files in the main program that starts the pipe).
> 
> Hmm right.  We???d need to insert a hook in ???open-pipe??? so that, when this
> behavior is wanted, one can actually close those FDs.

If I'm the only one concerned then I can (and actually already did)
handle the issue using the close-on-exec flag (as was suggested to me on
this list when I raised the problem initially).

I'm much more interested in working to solve the FD_SETSIZE limit and/or
the ports thread safety issue. :-)

Re: debugging guile runtime

[Andy Wingo]

Hi Cedric,

Let me first thank you for your patience!  It is excellent having
someone test the edges of Guile, and to stay around with us while we fix
them :-)

On Mon 29 Aug 2011 14:50, rixed@happyleptic.org writes:

> #1 ports are not thread safe (and any other thread safety issues) ;

In general I think this issue needs to be split up between issues with
port buffers and other issues; while it might be helpful to you to have
a tracker bug, it's not helpful to me to conflate things that require
different fixes.

So!  As you say, not thread-safe.  But can we fix it in 2.0?  I am not
sure.  We can add a mutex onto the end of scm_t_port.  But it seems that
ignoring ABI compatibility might allow us to focus on the solution more
easily.  What is your target?  How much are you willing to do yourself?

> #2 fork may freeze in some occurrence ;

I assume this is because of the port-table mutex bug that you posted
earlier?  We should be able to fix this with an atfork.

> #3 the use of select prevent the extended app to open more than 1024
>    files ;

I recall something about this; can you give a link to a bug?  If there
isn't one, can you file one?

> #4 fork does not close all open files.

This won't change in 2.0.  You can do something in an atfork, but... I'm
not sure this is the right thing.  The POSIX behavior was
well-considered, and we should be hesitant to change it without a good
reason.

> #5 new syntax definitions are not loaded by compiler

Hmm?

Cheers,

Andy
-- 
http://wingolog.org/

Re: debugging guile runtime

[Andy Wingo]

Are you saying that we should have an interface that makes sure that the
child process *only* has some set of file descriptors open?

Just trying to understand :)

Andy
-- 
http://wingolog.org/

Re: debugging guile runtime

[rixed]

> > #1 ports are not thread safe (and any other thread safety issues) ;
> 
> In general I think this issue needs to be split up between issues with
> port buffers and other issues; while it might be helpful to you to have
> a tracker bug, it's not helpful to me to conflate things that require
> different fixes.
> 
> So!  As you say, not thread-safe.  But can we fix it in 2.0?  I am not
> sure.  We can add a mutex onto the end of scm_t_port.  But it seems that
> ignoring ABI compatibility might allow us to focus on the solution more
> easily.

I can't see what solution you envision that requires breaking the
API. Are you thinking about a more functional style API for ports ?
I confess that the only solution I envisaged for now was merely to add a
global lock on all ports operations (I'm a little afraid about a
per-port mutex but that's certainly because I spend the last 2 days
hunting a race condition in my C program :-))

Anyway, as I'm not very familiar with the runtime I though you (or
someone who is) would suggest the best solution to me ; so I did not
try anything yet :-)

> What is your target?  How much are you willing to do yourself?

Ideally, I'd like the list to hint me toward a quick fix that I could
implement quickly, so that the problem is at least solved and the program
I work on can go to production, thus effectively testing the fix before
commit it into guile. The alternative being me rewriting some 15 lines
of scheme into 150 lines of C (and suffering sarcasm from my colleagues
:-))
I might have a week or so to devote to this matter before my team favor the
other solution (do it in C). A full week of time of paid work on the runtime!
Too bad I'm a guile and scheme newbie, so unfortunately it's probably roughly
equivalent to 2 hours for any of you.

> > #2 fork may freeze in some occurrence ;
> 
> I assume this is because of the port-table mutex bug that you posted
> earlier?  We should be able to fix this with an atfork.

I was referring to the problem I posted a while ago about this small
scheme program that was deadlocking in open-pipe:


----
(use-modules (ice-9 popen)
			 (ice-9 threads))

(define (repeat n f)
  (if (> n 0)
	  (begin
		(f)
		(repeat (- n 1) f))))

(define (forever f)
  (f)
  (forever f))

(display "Spawn a thread that performs some writes\n")
(make-thread forever (lambda ()
					   (display "write...\n")))

(display "Now exec some processes...\n")
(forever (lambda ()
		   (let ((pipe (open-input-pipe "sleep 0")))
			 (close-pipe pipe))))
---

I can't reproduce the bug with guile2, and honestly I can't say for sure
that I have seen it in the wild with guile 2 (although it's frequent
when the app runs with guile 1.8), but I have not performed many tests
with guile 2 yet. I'm about to upgrade one of our most used test server
with guile 2 to see how it behaves, so we will quickly know if it's
still relevant or not.

> > #3 the use of select prevent the extended app to open more than 1024
> >    files ;
> 
> I recall something about this; can you give a link to a bug?  If there
> isn't one, can you file one?

I did not filled a bug at savannah, but posted a patch here.
For the record, here is the only part of the patch that's still relevant
for v2.0.2:

diff --git a/libguile/fports.c b/libguile/fports.c
index 0b84d44..f19d291 100644
--- a/libguile/fports.c
+++ b/libguile/fports.c
@@ -49,7 +49,9 @@
 #ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
 #include <sys/stat.h>
 #endif
-
+#ifdef HAVE_POLL_H
+#include <poll.h>
+#endif
 #include <errno.h>
 #include <sys/types.h>
 
@@ -585,7 +587,14 @@ scm_fdes_to_port (int fdes, char *mode, SCM name)
 static int
 fport_input_waiting (SCM port)
 {
-#ifdef HAVE_SELECT
+#ifdef HAVE_POLL
+  int fdes = SCM_FSTREAM (port)->fdes;
+  struct pollfd pollfd = { fdes, POLLIN, 0 };
+  if (poll(&pollfd, 1, 0) < 0)
+    scm_syserror ("fport_input_waiting");
+  return pollfd.revents & POLLIN ? 1 : 0;
+
+#elif defined(HAVE_SELECT)
   int fdes = SCM_FSTREAM (port)->fdes;
   struct timeval timeout;
   SELECT_TYPE read_set;

Patch for 1.8 was much heavier though, since select was used here and there
within the runtime (by fport_wait_for_input which vanished and by scm_accept
that's not using select any more). With the above patch a guile2 user can open
more than 1024 files (as long as he does not call explicitly select of course).

I'm using this and it seams to work (again, not heavily tested with guile2).

> > #4 fork does not close all open files.
> 
> This won't change in 2.0.  You can do something in an atfork, but... I'm
> not sure this is the right thing.  The POSIX behavior was
> well-considered, and we should be hesitant to change it without a good
> reason.

Yes this was discussed already, I was wrong and the current behavior is
correct (yet I still think POSIX is weird here but that's another matter).

> > #5 new syntax definitions are not loaded by compiler
> 
> Hmm?

Also, already discussed. Mark convinced me that this is not a bug and I should
stop using load for loading code but use the module system instead.