* Re: bug#15228: [PATCH] Close output port of I/O pipes
[not found] <1377937797.2030.5.camel@qwghlm>
@ 2016-06-21 10:47 ` Andy Wingo
0 siblings, 0 replies; 3+ messages in thread
From: Andy Wingo @ 2016-06-21 10:47 UTC (permalink / raw)
To: Josep Portella Florit; +Cc: 15228, guile-devel
Hi :)
I dunno how much we should push this "processes are a single port"
abstraction. In many ways for OPEN_BOTH pipes it's easier to deal with
an input and an output port and a PID instead of the pipe abstraction.
WDYT? We could just expose `open-process' from (ice-9 popen) to start
with. It would be good to allow other fd's or ports to map to the child
as well, e.g. stderr or any particular port; but I don't know what
interface we should expose.
Thoughts?
Andy
On Sat 31 Aug 2013 10:29, Josep Portella Florit <jpf@primfilat.com> writes:
> There is a missing feature for pipes created with mode OPEN_BOTH:
>
> (use-modules (ice-9 popen))
> (use-modules (rnrs io ports))
>
> (let ((p (open-pipe "md5sum" OPEN_BOTH)))
> (put-string p "hello")
> (let ((x (get-string-all p)))
> (close-pipe p)
> x))
>
> This code deadlocks in get-string-all because md5sum, like other
> filters, keeps waiting for input until the pipe's output port is
> closed.
>
> The output port can't be closed without closing the input port too,
> because an I/O pipe is a soft port that doesn't store the 2 ports
> returned by open-process, but a thunk which closes both ports.
>
> This is now possible with the new procedure close-pipe-output:
>
> (let ((p (open-pipe "md5sum" OPEN_BOTH)))
> (put-string p "hello")
> (close-pipe-output p)
> (let ((x (get-string-all p)))
> (close-pipe p)
> x))
> ;; => "5d41402abc4b2a76b9719d911017c592 -\n"
>
> The intention is to make a backwards compatible and minimal change
> that makes it possible to write to and read from pipes for filters
> like md5sum without temporary files.
>
> Changes involved:
>
> * module/ice-9/popen.scm: Define a weak hash-table for mapping I/O pipes to
> their output ports, change make-rw-port to use it, define the
> close-pipe-output procedure and export it.
>
> * doc/ref/posix.texi: Add documentation for close-pipe-output.
>
> On garbage collection the new hash-table is updated as expected:
>
> scheme@(ice-9 popen)> rw/w-table
> $3 = #<weak-key-hash-table 8b8a930 0/31>
> scheme@(ice-9 popen)> (define p (open-pipe "md5sum" OPEN_BOTH))
> scheme@(ice-9 popen)> rw/w-table
> $4 = #<weak-key-hash-table 8b8a930 1/31>
> scheme@(ice-9 popen)> (set! p #f)
> scheme@(ice-9 popen)> (gc)
> scheme@(ice-9 popen)> rw/w-table
> $5 = #<weak-key-hash-table 8b8a930 0/31>
>
> Maybe there is a better name for the new procedure.
> ---
> doc/ref/posix.texi | 6 ++++++
> module/ice-9/popen.scm | 39 +++++++++++++++++++++++++++++----------
> 2 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/doc/ref/posix.texi b/doc/ref/posix.texi
> index b3a6a04..f0c6ca1 100644
> --- a/doc/ref/posix.texi
> +++ b/doc/ref/posix.texi
> @@ -2312,6 +2312,12 @@ terminate, and return the wait status code. The status is as per
> (@pxref{Processes})
> @end deffn
>
> +@deffn {Scheme Procedure} close-pipe-output port
> +Close the output port of a pipe created by @code{open-pipe} with
> +mode @code{OPEN_BOTH}, and leave the input port open. Return `#t' if
> +the port is closed successfully or `#f' if it was already closed.
> +@end deffn
> +
> @sp 1
> @code{waitpid WAIT_ANY} should not be used when pipes are open, since
> it can reap a pipe's child process, causing an error from a subsequent
> diff --git a/module/ice-9/popen.scm b/module/ice-9/popen.scm
> index 7d0549e..2b014c5 100644
> --- a/module/ice-9/popen.scm
> +++ b/module/ice-9/popen.scm
> @@ -18,22 +18,32 @@
> ;;;;
>
> (define-module (ice-9 popen)
> - :export (port/pid-table open-pipe* open-pipe close-pipe open-input-pipe
> - open-output-pipe open-input-output-pipe))
> + :export (port/pid-table open-pipe* open-pipe close-pipe close-pipe-output
> + open-input-pipe open-output-pipe open-input-output-pipe))
>
> (eval-when (load eval compile)
> (load-extension (string-append "libguile-" (effective-version))
> "scm_init_popen"))
>
> +;; a weak hash-table to store the write port of read-write pipes
> +;; just to be able to retrieve it in close-pipe-output.
> +(define rw/w-table (make-weak-key-hash-table 31))
> +
> (define (make-rw-port read-port write-port)
> - (make-soft-port
> - (vector
> - (lambda (c) (write-char c write-port))
> - (lambda (s) (display s write-port))
> - (lambda () (force-output write-port))
> - (lambda () (read-char read-port))
> - (lambda () (close-port read-port) (close-port write-port)))
> - "r+"))
> + (letrec ((port (make-soft-port
> + (vector
> + (lambda (c) (write-char c write-port))
> + (lambda (s) (display s write-port))
> + (lambda () (force-output write-port))
> + (lambda () (read-char read-port))
> + (lambda ()
> + (hashq-remove! rw/w-table port)
> + (close-port read-port)
> + (or (port-closed? write-port)
> + (close-port write-port))))
> + "r+")))
> + (hashq-set! rw/w-table port write-port)
> + port))
>
> ;; a guardian to ensure the cleanup is done correctly when
> ;; an open pipe is gc'd or a close-port is used.
> @@ -106,6 +116,15 @@ information on how to interpret this value."
> (error "close-pipe: pipe not in table"))
> (close-process (cons p pid))))
>
> +(define (close-pipe-output pipe)
> + "Closes the output port of a pipe created by @code{open-pipe} with
> +mode @code{OPEN_BOTH}, and leaves the input port open. Returns `#t' if
> +it successfully closes the port or `#f' if it was already closed."
> + (let ((port (hashq-ref rw/w-table pipe)))
> + (unless port
> + (error "close-pipe-output: pipe not in table"))
> + (close-port port)))
> +
> (define reap-pipes
> (lambda ()
> (let loop ((p (pipe-guardian)))
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: bug#15228: [PATCH] Close output port of I/O pipes
@ 2016-06-25 13:51 Josep Portella Florit
2016-06-25 15:49 ` Andy Wingo
0 siblings, 1 reply; 3+ messages in thread
From: Josep Portella Florit @ 2016-06-25 13:51 UTC (permalink / raw)
To: wingo; +Cc: guile-devel
> I dunno how much we should push this "processes are a single port"
> abstraction. In many ways for OPEN_BOTH pipes it's easier to deal with
> an input and an output port and a PID instead of the pipe abstraction.
> WDYT? We could just expose `open-process' from (ice-9 popen) to start
> with. It would be good to allow other fd's or ports to map to the child
> as well, e.g. stderr or any particular port; but I don't know what
> interface we should expose.
Since patching was inconvenient for me, I eventually used:
(use-modules ((ice-9 popen) #:select (open-process)))
Which works even though `open-process` is not exported. For me,
exporting `open-process` and documenting it would be enough.
I also like the Racket interface to processes:
<https://docs.racket-lang.org/reference/subprocess.html>
(I've mostly used the `process` procedure.)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: bug#15228: [PATCH] Close output port of I/O pipes
2016-06-25 13:51 Josep Portella Florit
@ 2016-06-25 15:49 ` Andy Wingo
0 siblings, 0 replies; 3+ messages in thread
From: Andy Wingo @ 2016-06-25 15:49 UTC (permalink / raw)
To: Josep Portella Florit; +Cc: guile-devel
On Sat 25 Jun 2016 15:51, Josep Portella Florit <jpf@primfilat.com> writes:
>> I dunno how much we should push this "processes are a single port"
>> abstraction. In many ways for OPEN_BOTH pipes it's easier to deal with
>> an input and an output port and a PID instead of the pipe abstraction.
>> WDYT? We could just expose `open-process' from (ice-9 popen) to start
>> with. It would be good to allow other fd's or ports to map to the child
>> as well, e.g. stderr or any particular port; but I don't know what
>> interface we should expose.
>
> Since patching was inconvenient for me, I eventually used:
>
> (use-modules ((ice-9 popen) #:select (open-process)))
>
> Which works even though `open-process` is not exported.
Note that this behavior of #:select is a bug. We won't remove it in
stable-2.0 but we have removed it in master.
> For me, exporting `open-process` and documenting it would be enough.
Fine with me, many people have asked for this at this point. I guess
that's the next step for this bug.
> I also like the Racket interface to processes:
> <https://docs.racket-lang.org/reference/subprocess.html>
> (I've mostly used the `process` procedure.)
Duly noted! The more we steal from Racket, the better Guile will be :)
Andy
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-25 15:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1377937797.2030.5.camel@qwghlm>
2016-06-21 10:47 ` bug#15228: [PATCH] Close output port of I/O pipes Andy Wingo
2016-06-25 13:51 Josep Portella Florit
2016-06-25 15:49 ` 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).