Mailing List Archive

Non Blocking File Streaming - write_fh
Hi all, and mostly John,

I was just browsing through the upcoming Advent code examples when I saw
the example for this had a bug. The code will die horribly when a call
is made to HEAD rather than get as the output filehandle gets closed.
The die is from an uncaught exception on $self->write.

My patch to MyApp::Stream :

sub read_chunk {
my ($self, $fh, $offset) = @_;
my $buffer = '';
aio_read $fh, $offset, 65536, $buffer, 0, sub {
my $status = shift;
die "read error[$status]: $!" unless $status >= 0;
if($status) {
eval {
$self->write($buffer);
};
if ($@) {
warn "Cannot write, probably a closed pipe";
}
$self->read_chunk($fh, ($offset + 65536));
} else {
$self->close;
aio_close $fh, sub { };
}
}
}

So the eval block and the warn, not die as this is not being caught
under any other exception handler. Real world, YMMV. Not elegant, but
would probably be better for passing in context.

Also I have a full Advent style writeup of the issue Bill brought up
recently with explainations and solutions down to making the handle DWIW
for for transparent handing over to Catalyst. If that is wanted/needed :)

Neil


---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com


_______________________________________________
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Re: Non Blocking File Streaming - write_fh [ In reply to ]
Neil,

You're the man!  Any change I could get this as a pull request on the github repo (is also the sane place I am asking people to submit articles)


https://github.com/perl-catalyst/2013-Advent-Staging

That way I am sure to get the code correct.

The only thing is that when using eval I prefer the following idiom as I think its a little safer

eval { ...; 1 } || die "trouble: $!";

On the other hand since Catalyst already requires Try::Tiny no reason to just not use that.

John



On Friday, December 6, 2013 1:56 AM, neil.lunn <neil@mylunn.id.au> wrote:

Hi all, and mostly John,

I was just browsing through the upcoming Advent code examples when I saw
the example for this had a bug. The code will die horribly when a call
is made to HEAD rather than get as the output filehandle gets closed.
The die is from an uncaught exception on $self->write.

My patch to MyApp::Stream :

sub read_chunk {
   my ($self, $fh, $offset) = @_;
   my $buffer = '';
   aio_read $fh, $offset, 65536, $buffer, 0, sub {
     my $status = shift;
     die "read error[$status]: $!" unless $status >= 0;
     if($status) {
       eval {
         $self->write($buffer);
       };
       if ($@) {
         warn "Cannot write, probably a closed pipe";
       }
       $self->read_chunk($fh, ($offset + 65536));
     } else {
       $self->close;
       aio_close $fh, sub { };
     }
   }
}

So the eval block and the warn, not die as this is not being caught
under any other exception handler. Real world, YMMV. Not elegant, but
would probably be better for passing in context.

Also I have a full Advent style writeup of the issue Bill brought up
recently with explainations and solutions down to making the handle DWIW
for for transparent handing over to Catalyst. If that is wanted/needed :)

Neil


---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com


_______________________________________________
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/

_______________________________________________
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/
Re: Non Blocking File Streaming - write_fh [ In reply to ]
On 7/12/2013 7:37 AM, John Napiorkowski wrote:
> Neil,
>
> You're the man! Any change I could get this as a pull request on the github repo (is also the sane place I am asking people to submit articles)
>
>
> https://github.com/perl-catalyst/2013-Advent-Staging

Sure. sending pull req now.
>
> That way I am sure to get the code correct.
>
> The only thing is that when using eval I prefer the following idiom as I think its a little safer
>
> eval { ...; 1 } || die "trouble: $!";
>
> On the other hand since Catalyst already requires Try::Tiny no reason to just not use that.

Missed that. But interestingly this does not seem to import in this context.

Also noted that this is just happening under Twiggy. This code seems to
be responsible:

package Twiggy::Writer;
use AnyEvent::Handle;
sub new {
my ( $class, $socket, $exit ) = @_;
bless { handle => AnyEvent::Handle->new( fh => $socket ), exit_guard =>
$exit }, $class;
}


Twiggy seems to be aware that the output socket has been closed where
some other server implementations do
not. Have had this problem before.

Strange IO::Handle in response body code there also MyApp-FunnyIO

Neil

>
> John
>
>
>
> On Friday, December 6, 2013 1:56 AM, neil.lunn <neil@mylunn.id.au> wrote:
>
> Hi all, and mostly John,
>
> I was just browsing through the upcoming Advent code examples when I saw
> the example for this had a bug. The code will die horribly when a call
> is made to HEAD rather than get as the output filehandle gets closed.
> The die is from an uncaught exception on $self->write.
>
> My patch to MyApp::Stream :
>
> sub read_chunk {
> my ($self, $fh, $offset) = @_;
> my $buffer = '';
> aio_read $fh, $offset, 65536, $buffer, 0, sub {
> my $status = shift;
> die "read error[$status]: $!" unless $status >= 0;
> if($status) {
> eval {
> $self->write($buffer);
> };
> if ($@) {
> warn "Cannot write, probably a closed pipe";
> }
> $self->read_chunk($fh, ($offset + 65536));
> } else {
> $self->close;
> aio_close $fh, sub { };
> }
> }
> }
>
> So the eval block and the warn, not die as this is not being caught
> under any other exception handler. Real world, YMMV. Not elegant, but
> would probably be better for passing in context.
>
> Also I have a full Advent style writeup of the issue Bill brought up
> recently with explainations and solutions down to making the handle DWIW
> for for transparent handing over to Catalyst. If that is wanted/needed :)
>
> Neil
>
>
> ---
> This email is free from viruses and malware because avast! Antivirus protection is active.
> http://www.avast.com
>
>
> _______________________________________________
> List: Catalyst@lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>
> _______________________________________________
> List: Catalyst@lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/



---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com