From: Andy Wingo <wingo@pobox.com>
To: Josep Portella Florit <jpf@primfilat.com>
Cc: 15228@debbugs.gnu.org, guile-devel@gnu.org
Subject: bug#15228: [PATCH] Close output port of I/O pipes
Date: Tue, 21 Jun 2016 12:47:38 +0200 [thread overview]
Message-ID: <87eg7q3jjp.fsf__18288.1386638202$1466506119$gmane$org@pobox.com> (raw)
In-Reply-To: <1377937797.2030.5.camel@qwghlm> (Josep Portella Florit's message of "Sat, 31 Aug 2013 10:29:57 +0200")
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)))
next prev parent reply other threads:[~2016-06-21 10:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-31 8:29 bug#15228: [PATCH] Close output port of I/O pipes Josep Portella Florit
2016-06-21 10:47 ` Andy Wingo [this message]
2016-09-03 7:33 ` bug#15228: making open-process public Amirouche Boubekki
2017-02-28 9:58 ` Andy Wingo
2017-03-05 20:41 ` Amirouche
2021-06-02 5:45 ` bug#15228: can this be closed ? Adriano Peluso
[not found] <628d9c7d-e7d5-6e55-1361-316629db8b4b@primfilat.com>
[not found] ` <8760sxb74x.fsf@pobox.com>
2016-06-27 8:05 ` bug#15228: [PATCH] Close output port of I/O pipes Andy Wingo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='87eg7q3jjp.fsf__18288.1386638202$1466506119$gmane$org@pobox.com' \
--to=wingo@pobox.com \
--cc=15228@debbugs.gnu.org \
--cc=guile-devel@gnu.org \
--cc=jpf@primfilat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).