Re: [Chicken-users] Spiffy subprocess cleanup
On Wed, May 22, 2013 at 10:29:03PM -0700, Evan Hanson wrote: After looking at a bit more, here's what I believe is *actually* happening: The invalid call to process* is signaling an exception in the child, which is handled internally by spiffy (spiffy.scm:470), causing that process to loop back to the start of the accept-next-connection procedure inside spiffy's accept-loop. At this point you have two processes listening on 8080, with the parent waiting for the child (who has no plans to exit). The same thing happens with each request, so the subprocesses pile up. Thanks for the excellent debugging work. I think process-fork really needs to catch exceptions and exit instead of simply exiting if the thunk returns normally. Will post a patch later, unless someone beats me to it. Cheers, Peter -- http://www.more-magic.net ___ Chicken-users mailing list Chicken-users@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-users
Re: [Chicken-users] Spiffy subprocess cleanup
On Thu, May 23, 2013 at 09:14:25AM +0200, Peter Bex wrote: On Wed, May 22, 2013 at 10:29:03PM -0700, Evan Hanson wrote: After looking at a bit more, here's what I believe is *actually* happening: The invalid call to process* is signaling an exception in the child, which is handled internally by spiffy (spiffy.scm:470), causing that process to loop back to the start of the accept-next-connection procedure inside spiffy's accept-loop. At this point you have two processes listening on 8080, with the parent waiting for the child (who has no plans to exit). The same thing happens with each request, so the subprocesses pile up. Thanks for the excellent debugging work. I think process-fork really needs to catch exceptions and exit instead of simply exiting if the thunk returns normally. Will post a patch later, unless someone beats me to it. Cheers, Peter Thanks a lot Evan and Peter. That was very helpful. I started looking at the process-fork source to write a patch, but I'm not familiar enough with chicken-core yet to tackle it. For now I changed my code to use the process* variant that sends a command to the shell and that is working. One step closer to using chicken/spiffy for new web development at my company. Wrapping up the first version of an egg to get at multipart/form-data in spiffy so I can accept file uploads. Bryan ___ Chicken-users mailing list Chicken-users@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-users
Re: [Chicken-users] Spiffy subprocess cleanup
On Thu, May 23, 2013 at 08:57:09AM -0700, Bryan Vicknair wrote: Thanks a lot Evan and Peter. That was very helpful. I started looking at the process-fork source to write a patch, but I'm not familiar enough with chicken-core yet to tackle it. For now I changed my code to use the process* variant that sends a command to the shell and that is working. I've attached a patch that I *think* does the trick. It also shoots down any additional threads which may be running in the child process (even though those should probably not get a chance to run, you never know). I've also simplified the argument handling and needless(?) messing about with process-fork without thunk in process-run. I'd love it if people could test this and provide feedback, because I'm pretty unsure whether this fix is the correct one. I've had some trouble trying to wait for the process, but I was unable to do that even if the program I tried to execute existed, so that must be a case of PEBKAC. Cheers, Peter -- http://www.more-magic.net From d4de7eb646e57e8c2d2e3d3649cf1a3ec1b65cd8 Mon Sep 17 00:00:00 2001 From: Peter Bex peter@xs4all.nl Date: Thu, 23 May 2013 21:15:19 +0200 Subject: [PATCH] Kill other threads when starting a process, and exit even when an exception occurs (reported by Bryan Vicknair, analysis by Evan Hanson) --- NEWS | 3 +++ posixunix.scm | 32 ++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index be9d098..c08ca60 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,9 @@ (thanks to Florian Zumbiehl) - posix: memory-mapped file support for Windows (thanks to rivo) - posix: find-file's test argument now also accepts SRE forms. + - posix: process, process* and process-run now properly kill other threads + and cause the process to exit with status 1 if running the process fails + (thanks to Evan Hanson and Bryan Vicknair) - Runtime system - Special events in poll() are now handled, avoiding hangs in threaded apps. diff --git a/posixunix.scm b/posixunix.scm index a2776da..7d8dcaf 100644 --- a/posixunix.scm +++ b/posixunix.scm @@ -1734,12 +1734,14 @@ EOF (when (fx= -1 pid) (posix-error #:process-error 'process-fork cannot create child process)) (if (and thunk (zero? pid)) - ((if killothers -##sys#kill-other-threads -(lambda (thunk) (thunk))) -(lambda () - (thunk) - ((foreign-lambda void _exit int) 0) )) + (handle-exceptions exn + ((foreign-lambda void _exit int) 1) + ((if killothers +##sys#kill-other-threads +(lambda (thunk) (thunk))) +(lambda () + (thunk) + ((foreign-lambda void _exit int) 0) ))) pid) (define process-execute @@ -1816,13 +1818,14 @@ EOF (list -c cmdlin) ) (define process-run - (lambda (f . args) -(let ([args (if (pair? args) (car args) #f)] - [pid (process-fork)] ) - (cond [(not (eq? 0 pid)) pid] - [args (process-execute f args)] - [else -(process-execute (##sys#shell-command) (##sys#shell-command-arguments f)) ] ) ) ) ) + (lambda (f #!optional args) +(process-fork + (lambda () + (if args + (process-execute f args) + (process-execute (##sys#shell-command) +(##sys#shell-command-arguments f)) )) + #t) ) ) ;;; Run subprocess connected with pipes: @@ -1893,7 +1896,8 @@ EOF (connect-child loc opipe stdinf fileno/stdin) (connect-child loc (swapped-ends ipipe) stdoutf fileno/stdout) (connect-child loc (swapped-ends epipe) stderrf fileno/stderr) -(process-execute cmd args env ) ) )] +(process-execute cmd args env)) + #t)) ) ) )] [input-port (lambda (loc pid cmd pipe stdf stdfd on-close) (and-let* ([fd (connect-parent loc pipe stdf stdfd)]) -- 1.8.2.3 ___ Chicken-users mailing list Chicken-users@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-users
[Chicken-users] Spiffy subprocess cleanup
I have a web app using spiffy that uses (process*) in one of the views to wait on a script to do some work. I was accidentally passing in a non-existent script name to process*, which was causing a 500 code to be returned by spiffy. The problem is that there is no indication of any error sent to stderr/stdout by spiffy. Also, each time I visit the broken view's URL, a new subprocess is created and never killed. Here is an example: (use posix spiffy) (define (fail) (define-values (i o pid e) (process* non-existant '(arg1 arg2))) (close-output-port o) (close-input-port i) (close-input-port e)) (define (server arg) (print starting subprocess...) (fail) (send-status ok done)) (access-log (current-output-port)) (error-log (current-error-port)) (debug-log (current-output-port)) (handle-not-found server) (root-path ./) (start-server) If you save the above code in foo.scm, compile and run it, then go to http://localhost:8080/bla a few times so the handle-not-found handler is called, you'll get a 500 error. But the shell you started foo in won't show any details on the error. If you look at pstree, you'll see that there are subprocesses hanging around: sh--bash--foo-foo-foo-foo I thought there might be strange interplay between the thread that spiffy is using to serve the view and the (process*) call. The example below doesn't involve spiffy, it just starts a thread that will fail by calling (process*) with a non-existent script name: (use srfi-18 posix) (define (fail) (define-values (i o pid e) (process* non-existant '(arg1 arg2))) (close-output-port o) (close-input-port i) (close-input-port e)) (print staring thread that will fail) (thread-start! (make-thread (lambda() (thread-sleep! 1) (fail (thread-sleep! 2) (print main thread done) This works as expected however. There are no subprocesses lying around, and the error appears on stderr. Does anyone understand why the (process*) call in the spiffy example doesn't show an error on stderr/stdout and leaves subprocesses alive? Bryan ___ Chicken-users mailing list Chicken-users@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-users
Re: [Chicken-users] Spiffy subprocess cleanup
Hi Bryan, I tried to paste a response but missed you in #chicken. I think you need to make sure to close the ports returned by process* (in your paste, the close-input/output-port forms aren't running since process* is signaling an error): http://paste.call-cc.org/paste?id=76432a008b2c4310d0044e91d4bcf48c74b730d8#a1 You can register an exception handler for spiffy to make it more clear what/when things are going wrong: http://api.call-cc.org/doc/spiffy#def:handle-exception Cheers, Evan ___ Chicken-users mailing list Chicken-users@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-users
Re: [Chicken-users] Spiffy subprocess cleanup
After looking at a bit more, here's what I believe is *actually* happening: The invalid call to process* is signaling an exception in the child, which is handled internally by spiffy (spiffy.scm:470), causing that process to loop back to the start of the accept-next-connection procedure inside spiffy's accept-loop. At this point you have two processes listening on 8080, with the parent waiting for the child (who has no plans to exit). The same thing happens with each request, so the subprocesses pile up. Evan ___ Chicken-users mailing list Chicken-users@nongnu.org https://lists.nongnu.org/mailman/listinfo/chicken-users
