Skip Menu |

This queue is for tickets about the IO-Async CPAN distribution.

Report information
The Basics
Id: 115920
Status: resolved
Priority: 0/
Queue: IO-Async

People
Owner: Nobody in particular
Requestors: leonerd-cpan [...] leonerd.org.uk
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: 0.70
Fixed in: 0.71



Subject: Hook to allow modification of IaProcess handles before fork
E.g. useful to setsockopt on socketpairs. Might be written like via => "socketpair", prefork => sub { my ( $myfh, $childfh ) = @_; ... }, -- Paul Evans
RT-Send-CC: leonerd-cpan [...] leonerd.org.uk
Here's a preliminary patch - I can do more tests and add POD documentation if this is in generally the direction you'd like this to go.
Subject: sockopt.diff
Common subdirectories: IO-Async-0.70/examples and IO-Async-0.70-sockopt//examples Common subdirectories: IO-Async-0.70/lib and IO-Async-0.70-sockopt//lib Common subdirectories: IO-Async-0.70/t and IO-Async-0.70-sockopt//t Common subdirectories: IO-Async-0.70/lib/IO and IO-Async-0.70-sockopt//lib/IO Common subdirectories: IO-Async-0.70/lib/IO/Async and IO-Async-0.70-sockopt//lib/IO/Async Only in IO-Async-0.70-sockopt//lib/IO/Async: .Notifier.pm.swp Only in IO-Async-0.70-sockopt//lib/IO/Async: .Process.pm.swp Common subdirectories: IO-Async-0.70/lib/IO/Async/Internals and IO-Async-0.70-sockopt//lib/IO/Async/Internals Common subdirectories: IO-Async-0.70/lib/IO/Async/Loop and IO-Async-0.70-sockopt//lib/IO/Async/Loop Common subdirectories: IO-Async-0.70/lib/IO/Async/OS and IO-Async-0.70-sockopt//lib/IO/Async/OS diff -ru IO-Async-0.70/lib/IO/Async/Process.pm IO-Async-0.70-sockopt//lib/IO/Async/Process.pm --- IO-Async-0.70/lib/IO/Async/Process.pm Tue Dec 15 13:19:16 2015 +++ IO-Async-0.70-sockopt//lib/IO/Async/Process.pm Wed Jul 6 20:26:06 2016 @@ -262,7 +262,7 @@ # All these parameters can only be configured while the process isn't # running my %setup_params; - foreach (qw( code command setup stdin stdout stderr stdio ), grep { m/^fd\d+$/ } keys %params ) { + foreach (qw( code command setup stdin stdout stderr stdio prefork ), grep { m/^fd\d+$/ } keys %params ) { $setup_params{$_} = delete $params{$_} if exists $params{$_}; } @@ -274,7 +274,7 @@ defined( exists $setup_params{command} ? $setup_params{command} : $self->{command} ) <= 1 or croak "Cannot have both 'code' and 'command'"; - foreach (qw( code command setup )) { + foreach (qw( code command setup prefork )) { $self->{$_} = delete $setup_params{$_} if exists $setup_params{$_}; } @@ -438,6 +438,8 @@ elsif( $via == FD_VIA_SOCKETPAIR ) { my ( $myfd, $childfd ) = IO::Async::OS->socketpair( $opts->{family}, $opts->{socktype} ) or croak "Unable to socketpair() - $!"; + $self->{prefork}->( $myfd, $childfd ) if exists($self->{prefork}); + $handle->configure( handle => $myfd ); if( $key eq "stdio" ) { Common subdirectories: IO-Async-0.70/lib/IO/Async/Protocol and IO-Async-0.70-sockopt//lib/IO/Async/Protocol Common subdirectories: IO-Async-0.70/lib/IO/Async/Timer and IO-Async-0.70-sockopt//lib/IO/Async/Timer Only in IO-Async-0.70-sockopt//t: .34process-handles.t.swp diff -ru IO-Async-0.70/t/34process-handles.t IO-Async-0.70-sockopt//t/34process-handles.t --- IO-Async-0.70/t/34process-handles.t Tue Dec 15 13:19:16 2015 +++ IO-Async-0.70-sockopt//t/34process-handles.t Wed Jul 6 20:34:57 2016 @@ -14,7 +14,7 @@ plan skip_all => "POSIX fork() is not available" unless IO::Async::OS->HAVE_POSIX_FORK; -use Socket qw( PF_INET sockaddr_family ); +use Socket qw( PF_INET sockaddr_family SOL_SOCKET SO_RCVBUF ); my $loop = IO::Async::Loop->new_builtin; @@ -333,6 +333,17 @@ send STDOUT, $pkt, 0 or die "Cannot send - $!"; return 0; }, + prefork => sub { + my ($myfd, $childfd) = @_; + + $myfd->setsockopt( SOL_SOCKET, SO_RCVBUF, 3 * 1024 * 1024); + $childfd->setsockopt(SOL_SOCKET, SO_RCVBUF, 3 * 1024 * 1024); + + cmp_ok($myfd->getsockopt( SOL_SOCKET, SO_RCVBUF), '==', 3 * 1024 * 1024, + 'my socket receive buffer change to 3 MB successful'); + cmp_ok($childfd->getsockopt(SOL_SOCKET, SO_RCVBUF), '==', 3 * 1024 * 1024, + 'child socket receive buffer change to 3 MB successful') + }, stdio => { via => "socketpair" }, on_finish => sub { }, ); @@ -355,6 +366,9 @@ isa_ok( $process->stdio->read_handle, "IO::Socket", '$process->stdio handle isa IO::Socket' ); + cmp_ok( $process->stdio->read_handle->getsockopt(SOL_SOCKET, SO_RCVBUF), + '==', 3 * 1024 * 1024, 'my socket receive buffer is still 3 MB'); + wait_for { defined $output_packet and !$process->is_running }; ok( $process->is_exited, '$process->is_exited after perl STDIO via socketpair' );
On Wed Jul 06 20:41:08 2016, GMARLER wrote: Show quoted text
> Here's a preliminary patch - I can do more tests and add POD > documentation if this is in generally the direction you'd like this to > go.
Ah, sortof. You've implemented it as an option on the IaProcess object itself. I was thinking it would be a parameter of the per-filehandle setup; e.g. IaProcess->new( command => [...], stdio => { via => "socketpair", prefork => sub { ... }, }, ) That way you could set different options on a per-filehandle basis; there might in general be more than one. Also I'd be a little wary of just testing using SO_RCVBUF without first guarding it with a SKIP block, that the OS it's running on even supports SO_RCVBUF in the first place. (E.g. does Windows support it?) A better test might be something a bit simpler - maybe just capture the filehandle arguments and test afterwards that they match? Or additionally, have the prefork code inject some content via `->write` that we can later test was there. -- Paul Evans
Show quoted text
> > You've implemented it as an option on the IaProcess object itself. I > was thinking it would be a parameter of the per-filehandle setup; e.g. > > IaProcess->new( > command => [...], > stdio => { > via => "socketpair", > prefork => sub { ... }, > }, > )
Point taken - have reimplemented it that way, and it was actually easier. Show quoted text
> > Also I'd be a little wary of just testing using SO_RCVBUF without > first guarding it with a SKIP block, that the OS it's running on even > supports SO_RCVBUF in the first place. (E.g. does Windows support it?) >
I'm searching now for how to determine if something a module could export is actually exported or not (like SO_RCVBUF), then using a SKIP block to avoid testing anything unless Socket::SO_RCVBUF and Socket::SOL_SOCKET exist. Show quoted text
> A better test might be something a bit simpler - maybe just capture > the filehandle arguments and test afterwards that they match? Or > additionally, have the prefork code inject some content via `->write` > that we can later test was there.
Not quite sure I follow you on this advice - please provide a few more details of: - which fh args to test? - inject some content into *what* via '->write' in the prefork code? Thanks!
On Fri Jul 08 17:54:39 2016, GMARLER wrote: Show quoted text
> I'm searching now for how to determine if something a module could > export is actually exported or not (like SO_RCVBUF), then using a SKIP > block to avoid testing anything unless Socket::SO_RCVBUF and > Socket::SOL_SOCKET exist.
I wouldn't bother. See below. ... Show quoted text
> Not quite sure I follow you on this advice - please provide a few more > details of: > > - which fh args to test? > - inject some content into *what* via '->write' in the prefork code?
In prefork, $fh->write( "some test data" ); Then once the IaProcess is started, check that the filehandle is now readable and that reading it contains that test data you just injected. -- Paul Evans
On Tue Jul 12 12:21:37 2016, PEVANS wrote: Show quoted text
> On Fri Jul 08 17:54:39 2016, GMARLER wrote:
> > I'm searching now for how to determine if something a module could > > export is actually exported or not (like SO_RCVBUF), then using a > > SKIP > > block to avoid testing anything unless Socket::SO_RCVBUF and > > Socket::SOL_SOCKET exist.
> > I wouldn't bother. See below. > > ...
> > Not quite sure I follow you on this advice - please provide a few > > more > > details of: > > > > - which fh args to test? > > - inject some content into *what* via '->write' in the prefork code?
> > In prefork, $fh->write( "some test data" ); > > Then once the IaProcess is started, check that the filehandle is now > readable and that reading it contains that test data you just > injected.
Oh, your suggestion was too simple for me - I overthought it. Thanks!
Here's the result, with POD updated with API change and an example.
Subject: sockopt-try2.diff
Common subdirectories: IO-Async-0.70/examples and IO-Async-0.70-sockopt//examples Common subdirectories: IO-Async-0.70/lib and IO-Async-0.70-sockopt//lib Common subdirectories: IO-Async-0.70/t and IO-Async-0.70-sockopt//t Common subdirectories: IO-Async-0.70/lib/IO and IO-Async-0.70-sockopt//lib/IO Common subdirectories: IO-Async-0.70/lib/IO/Async and IO-Async-0.70-sockopt//lib/IO/Async Only in IO-Async-0.70-sockopt//lib/IO/Async: .Process.pm.swp Common subdirectories: IO-Async-0.70/lib/IO/Async/Internals and IO-Async-0.70-sockopt//lib/IO/Async/Internals Common subdirectories: IO-Async-0.70/lib/IO/Async/Loop and IO-Async-0.70-sockopt//lib/IO/Async/Loop Common subdirectories: IO-Async-0.70/lib/IO/Async/OS and IO-Async-0.70-sockopt//lib/IO/Async/OS diff -ru IO-Async-0.70/lib/IO/Async/Process.pm IO-Async-0.70-sockopt//lib/IO/Async/Process.pm --- IO-Async-0.70/lib/IO/Async/Process.pm Tue Dec 15 13:19:16 2015 +++ IO-Async-0.70-sockopt//lib/IO/Async/Process.pm Tue Jul 12 18:15:58 2016 @@ -14,6 +14,7 @@ use Carp; use Socket qw( SOCK_STREAM ); +use Data::Dumper; use Future; @@ -232,6 +233,14 @@ C<from> parameter will be written to the child. When all of the data has been written the pipe will be closed. +=item prefork => CODE + +Only valid for via => 'socketpair', this code block runs after the +C<socketpair(2)> is created, but before the child is forked off. This is handy +for when you need to use C<setsockopt(3)> to condition both ends of the socket. +The arguments passed in are the L<IO::Socket> objects for the parent and child +ends of the socket. + =back =head2 stdin => ... @@ -355,7 +364,7 @@ } if( defined $via and $via == FD_VIA_SOCKETPAIR ) { - $self->{fd_opts}{$fd}{$_} = delete $args{$_} for qw( family socktype ); + $self->{fd_opts}{$fd}{$_} = delete $args{$_} for qw( family socktype prefork ); } keys %args and croak "Unexpected extra keys for fd $fd - " . join ", ", keys %args; @@ -438,6 +447,8 @@ elsif( $via == FD_VIA_SOCKETPAIR ) { my ( $myfd, $childfd ) = IO::Async::OS->socketpair( $opts->{family}, $opts->{socktype} ) or croak "Unable to socketpair() - $!"; + $opts->{prefork}->( $myfd, $childfd ) if $opts->{prefork}; + $handle->configure( handle => $myfd ); if( $key eq "stdio" ) { @@ -866,6 +877,29 @@ $process->stdin->write( "Here is some more data\n" ); +=head2 Setting socket options + +By using the C<prefork> code sub, you can change the socket receive buffer size +at both ends of the socket before the child is forked (at which point it would +be too late to change the child end of the socket). + + use Socket qw( SOL_SOCKET SO_RCVBUF ); + + my $process = IO::Async::Process->new( + command => [ "command-to-read-from-write-to", "arguments" ], + stdio => { via => "socketpair" }, + prefork => sub { + my ($parentfd, $childfd) = @_; + + # Set parent end of socket receive buffer to 3 MB + $parentfd->setsockopt(SOL_SOCKET, SO_RCVBUF, 3 * 1024 * 1024); + # Set child end of socket receive buffer to 3 MB + $childfd ->setsockopt(SOL_SOCKET, SO_RCVBUF, 3 * 1024 * 1024); + }, + ); + + $loop->add( $process ); + =cut =head1 AUTHOR Common subdirectories: IO-Async-0.70/lib/IO/Async/Protocol and IO-Async-0.70-sockopt//lib/IO/Async/Protocol Common subdirectories: IO-Async-0.70/lib/IO/Async/Timer and IO-Async-0.70-sockopt//lib/IO/Async/Timer Only in IO-Async-0.70-sockopt//t: .34process-handles.t.swp diff -ru IO-Async-0.70/t/34process-handles.t IO-Async-0.70-sockopt//t/34process-handles.t --- IO-Async-0.70/t/34process-handles.t Tue Dec 15 13:19:16 2015 +++ IO-Async-0.70-sockopt//t/34process-handles.t Tue Jul 12 17:41:15 2016 @@ -365,6 +365,48 @@ { my $process = IO::Async::Process->new( + code => sub { + defined( recv STDIN, my $pkt, 8192, 0 ) or die "Cannot recv - $!"; + send STDOUT, $pkt, 0 or die "Cannot send - $!"; + return 0; + }, + stdio => { + via => "socketpair", + prefork => sub { + my ($myfd, $childfd) = @_; + + $myfd->write("Data from the prefork"); + }, + }, + on_finish => sub { }, + ); + + isa_ok( $process->stdio, "IO::Async::Stream", '$process->stdio isa Stream' ); + + my $output_packet = ""; + $process->stdio->configure( + on_read => sub { + my ( undef, $buffref ) = @_; + $output_packet .= $$buffref; + $$buffref = ""; + return 0; + }, + ); + + $loop->add( $process ); + + isa_ok( $process->stdio->read_handle, "IO::Socket", '$process->stdio handle isa IO::Socket' ); + + wait_for { defined $output_packet and !$process->is_running }; + + ok( $process->is_exited, '$process->is_exited after perl STDIO via socketpair' ); + is( $process->exitstatus, 0, '$process->exitstatus after perl STDIO via socketpair' ); + + is_deeply( $output_packet, "Data from the prefork", '$output_packet from prefork via socketpair' ); +} + +{ + my $process = IO::Async::Process->new( code => sub { return 0 }, stdio => { via => "socketpair", family => "inet" }, on_finish => sub { },
Sorry, forget that last - MAJOR typo in the example.
Ok, finally - 3rd try.
Subject: sockopt-try3.diff
Common subdirectories: IO-Async-0.70/examples and IO-Async-0.70-sockopt//examples Common subdirectories: IO-Async-0.70/lib and IO-Async-0.70-sockopt//lib Common subdirectories: IO-Async-0.70/t and IO-Async-0.70-sockopt//t Common subdirectories: IO-Async-0.70/lib/IO and IO-Async-0.70-sockopt//lib/IO Common subdirectories: IO-Async-0.70/lib/IO/Async and IO-Async-0.70-sockopt//lib/IO/Async Only in IO-Async-0.70-sockopt//lib/IO/Async: .Process.pm.swp Common subdirectories: IO-Async-0.70/lib/IO/Async/Internals and IO-Async-0.70-sockopt//lib/IO/Async/Internals Common subdirectories: IO-Async-0.70/lib/IO/Async/Loop and IO-Async-0.70-sockopt//lib/IO/Async/Loop Common subdirectories: IO-Async-0.70/lib/IO/Async/OS and IO-Async-0.70-sockopt//lib/IO/Async/OS diff -ru IO-Async-0.70/lib/IO/Async/Process.pm IO-Async-0.70-sockopt//lib/IO/Async/Process.pm --- IO-Async-0.70/lib/IO/Async/Process.pm Tue Dec 15 13:19:16 2015 +++ IO-Async-0.70-sockopt//lib/IO/Async/Process.pm Tue Jul 12 18:24:31 2016 @@ -14,6 +14,7 @@ use Carp; use Socket qw( SOCK_STREAM ); +use Data::Dumper; use Future; @@ -232,6 +233,14 @@ C<from> parameter will be written to the child. When all of the data has been written the pipe will be closed. +=item prefork => CODE + +Only valid for via => 'socketpair', this code block runs after the +C<socketpair(2)> is created, but before the child is forked off. This is handy +for when you need to use C<setsockopt(3)> to condition both ends of the socket. +The arguments passed in are the L<IO::Socket> objects for the parent and child +ends of the socket. + =back =head2 stdin => ... @@ -355,7 +364,7 @@ } if( defined $via and $via == FD_VIA_SOCKETPAIR ) { - $self->{fd_opts}{$fd}{$_} = delete $args{$_} for qw( family socktype ); + $self->{fd_opts}{$fd}{$_} = delete $args{$_} for qw( family socktype prefork ); } keys %args and croak "Unexpected extra keys for fd $fd - " . join ", ", keys %args; @@ -438,6 +447,8 @@ elsif( $via == FD_VIA_SOCKETPAIR ) { my ( $myfd, $childfd ) = IO::Async::OS->socketpair( $opts->{family}, $opts->{socktype} ) or croak "Unable to socketpair() - $!"; + $opts->{prefork}->( $myfd, $childfd ) if $opts->{prefork}; + $handle->configure( handle => $myfd ); if( $key eq "stdio" ) { @@ -866,6 +877,31 @@ $process->stdin->write( "Here is some more data\n" ); +=head2 Setting socket options + +By using the C<prefork> code sub, you can change the socket receive buffer size +at both ends of the socket before the child is forked (at which point it would +be too late to change the child end of the socket). + + use Socket qw( SOL_SOCKET SO_RCVBUF ); + + my $process = IO::Async::Process->new( + command => [ "command-to-read-from-and-write-to", "arguments" ], + stdio => { + via => "socketpair", + prefork => sub { + my ($parentfd, $childfd) = @_; + + # Set parent end of socket receive buffer to 3 MB + $parentfd->setsockopt(SOL_SOCKET, SO_RCVBUF, 3 * 1024 * 1024); + # Set child end of socket receive buffer to 3 MB + $childfd ->setsockopt(SOL_SOCKET, SO_RCVBUF, 3 * 1024 * 1024); + }, + }, + ); + + $loop->add( $process ); + =cut =head1 AUTHOR Common subdirectories: IO-Async-0.70/lib/IO/Async/Protocol and IO-Async-0.70-sockopt//lib/IO/Async/Protocol Common subdirectories: IO-Async-0.70/lib/IO/Async/Timer and IO-Async-0.70-sockopt//lib/IO/Async/Timer Only in IO-Async-0.70-sockopt//t: .34process-handles.t.swp diff -ru IO-Async-0.70/t/34process-handles.t IO-Async-0.70-sockopt//t/34process-handles.t --- IO-Async-0.70/t/34process-handles.t Tue Dec 15 13:19:16 2015 +++ IO-Async-0.70-sockopt//t/34process-handles.t Tue Jul 12 17:41:15 2016 @@ -365,6 +365,48 @@ { my $process = IO::Async::Process->new( + code => sub { + defined( recv STDIN, my $pkt, 8192, 0 ) or die "Cannot recv - $!"; + send STDOUT, $pkt, 0 or die "Cannot send - $!"; + return 0; + }, + stdio => { + via => "socketpair", + prefork => sub { + my ($myfd, $childfd) = @_; + + $myfd->write("Data from the prefork"); + }, + }, + on_finish => sub { }, + ); + + isa_ok( $process->stdio, "IO::Async::Stream", '$process->stdio isa Stream' ); + + my $output_packet = ""; + $process->stdio->configure( + on_read => sub { + my ( undef, $buffref ) = @_; + $output_packet .= $$buffref; + $$buffref = ""; + return 0; + }, + ); + + $loop->add( $process ); + + isa_ok( $process->stdio->read_handle, "IO::Socket", '$process->stdio handle isa IO::Socket' ); + + wait_for { defined $output_packet and !$process->is_running }; + + ok( $process->is_exited, '$process->is_exited after perl STDIO via socketpair' ); + is( $process->exitstatus, 0, '$process->exitstatus after perl STDIO via socketpair' ); + + is_deeply( $output_packet, "Data from the prefork", '$output_packet from prefork via socketpair' ); +} + +{ + my $process = IO::Async::Process->new( code => sub { return 0 }, stdio => { via => "socketpair", family => "inet" }, on_finish => sub { },
On Tue Jul 12 18:25:21 2016, GMARLER wrote: Show quoted text
> Ok, finally - 3rd try.
Hi, Sorry it took a while to get around to this, but it's now applied in source. 1393: Paul "LeoNerd" Evans 2016-07-29 Minor wording and formatting fixes to applied patch 1392: Paul "LeoNerd" Evans 2016-07-29 Apply patch for RT#115920 from GMARLER -- Paul Evans