Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix a lot of timing bugs
  • Loading branch information
kraih committed Jan 11, 2016
1 parent 355b925 commit cce3b85
Show file tree
Hide file tree
Showing 13 changed files with 77 additions and 193 deletions.
5 changes: 4 additions & 1 deletion Changes
Expand Up @@ -2,6 +2,7 @@
6.40 2016-01-11
- Removed client_challenge, client_handshake and server_handshake methods from
Mojo::Transaction::WebSocket. (batman)
- Removed is_writing method from Mojo::Transaction.
- Removed upgrade event from Mojo::Transaction::HTTP.
- Deprecated Mojo::Transaction::WebSocket::build_frame and
Mojo::Transaction::WebSocket::parse_frame in favor of
Expand All @@ -11,8 +12,10 @@
- Added SNI support to all built-in web servers. (bpmedley, sri)
- Added module Mojo::WebSocket. (batman)
- Updated jQuery to version 2.2.0.
- Improve mtime attribute in Mojo::Asset::Memory to default to the value of
- Improved performance of Mojo::Server::Daemon and Mojo::UserAgent slightly.
- Improved mtime attribute in Mojo::Asset::Memory to default to the value of
$^T.
- Fixed a few timing bugs.
- Fixed url_for to handle fragments correctly.

6.39 2016-01-03
Expand Down
6 changes: 2 additions & 4 deletions lib/Mojo/Content.pm
Expand Up @@ -38,8 +38,7 @@ sub clone {
sub generate_body_chunk {
my ($self, $offset) = @_;

$self->emit(drain => $offset)
if !delete $self->{delay} && ($self->{body_buffer} // '') eq '';
$self->emit(drain => $offset) if ($self->{body_buffer} // '') eq '';
my $chunk = delete $self->{body_buffer} // '';
return $self->{eof} ? '' : undef if $chunk eq '';

Expand Down Expand Up @@ -148,8 +147,7 @@ sub write {
my ($self, $chunk, $cb) = @_;

$self->{dynamic} = 1;
if (defined $chunk) { $self->{body_buffer} .= $chunk }
else { $self->{delay} = 1 }
$self->{body_buffer} .= $chunk if defined $chunk;
$self->once(drain => $cb) if $cb;
$self->{eof} = 1 if defined $chunk && $chunk eq '';

Expand Down
20 changes: 9 additions & 11 deletions lib/Mojo/Server/Daemon.pm
Expand Up @@ -93,7 +93,13 @@ sub _build_tx {
# HTTP
else { $self->emit(request => $tx) }

# Last keep-alive request or corrupted connection
my $c = $self->{connections}{$id};
$tx->res->headers->connection('close')
if $c->{requests} >= $self->max_requests || $tx->req->error;

$tx->on(resume => sub { $self->_write($id) });
$self->_write($id);
}
);

Expand Down Expand Up @@ -206,14 +212,6 @@ sub _read {
my $tx = $c->{tx} ||= $self->_build_tx($id, $c);
warn term_escape "-- Server <<< Client (@{[_url($tx)]})\n$chunk\n" if DEBUG;
$tx->server_read($chunk);

# Last keep-alive request or corrupted connection
$tx->res->headers->connection('close')
if $c->{requests} >= $self->max_requests || $tx->req->error;

# Finish or start writing
if ($tx->is_finished) { $self->_finish($id) }
elsif ($tx->is_writing) { $self->_write($id) }
}

sub _remove {
Expand All @@ -229,17 +227,17 @@ sub _write {

# Get chunk and write
my $c = $self->{connections}{$id};
return unless my $tx = $c->{tx};
return if !$tx->is_writing || $c->{writing}++;
return if !(my $tx = $c->{tx}) || $c->{writing}++;
my $chunk = $tx->server_write;
delete $c->{writing};
warn term_escape "-- Server >>> Client (@{[_url($tx)]})\n$chunk\n" if DEBUG;
my $stream = $self->ioloop->stream($id)->write($chunk);

# Finish or continue writing
my $next = '_write';
my $next = $chunk eq '' ? undef : '_write';
$tx->has_subscribers('finish') ? ($next = '_finish') : $self->_finish($id)
if $tx->is_finished;
return unless $next;
weaken $self;
$stream->write('' => sub { $self->$next($id) });
}
Expand Down
8 changes: 0 additions & 8 deletions lib/Mojo/Transaction.pm
Expand Up @@ -27,8 +27,6 @@ sub is_finished { (shift->{state} // '') eq 'finished' }

sub is_websocket {undef}

sub is_writing { (shift->{state} // 'write') eq 'write' }

sub remote_address {
my $self = shift;

Expand Down Expand Up @@ -220,12 +218,6 @@ Check if transaction is finished.
False, this is not a L<Mojo::Transaction::WebSocket> object.
=head2 is_writing
my $bool = $tx->is_writing;
Check if transaction is writing.
=head2 resume
$tx = $tx->resume;
Expand Down
6 changes: 2 additions & 4 deletions lib/Mojo/Transaction/HTTP.pm
Expand Up @@ -87,17 +87,15 @@ sub _body {
my $written = defined $buffer ? length $buffer : 0;
$self->{write} = $msg->content->is_dynamic ? 1 : ($self->{write} - $written);
$self->{offset} += $written;
if (defined $buffer) { delete $self->{delay} }

# Delayed
elsif (delete $self->{delay}) { $self->{state} = 'read' }
else { $self->{delay} = 1 }
$self->{state} = 'read' unless defined $buffer;

# Finished
$self->{state} = $finish ? 'finished' : 'read'
if $self->{write} <= 0 || defined $buffer && $buffer eq '';

return defined $buffer ? $buffer : '';
return $buffer // '';
}

sub _headers {
Expand Down
3 changes: 1 addition & 2 deletions lib/Mojo/UserAgent.pm
Expand Up @@ -316,12 +316,11 @@ sub _write {

# Get and write chunk
my $c = $self->{connections}{$id};
return if $c->{writing}++ || !(my $tx = $c->{tx});
return if !(my $tx = $c->{tx}) || $c->{writing}++;
my $chunk = $tx->client_write;
delete $c->{writing};
warn term_escape "-- Client >>> Server (@{[_url($tx)]})\n$chunk\n" if DEBUG;
my $stream = $c->{ioloop}->stream($id)->write($chunk);
$self->_finish($id) if $tx->is_finished;

# Continue writing
return if $chunk eq '';
Expand Down
2 changes: 1 addition & 1 deletion lib/Mojolicious.pm
Expand Up @@ -127,7 +127,7 @@ sub handler {

# Delayed response
$self->log->debug('Nothing has been rendered, expecting delayed response')
unless $c->tx->is_writing;
unless $c->stash->{'mojo.rendered'};
}

sub helper { shift->renderer->add_helper(@_) }
Expand Down
4 changes: 0 additions & 4 deletions lib/Mojolicious/Controller.pm
Expand Up @@ -347,14 +347,12 @@ sub validation {

sub write {
my ($self, $chunk, $cb) = @_;
($cb, $chunk) = ($chunk, undef) if ref $chunk eq 'CODE';
$self->res->content->write($chunk, $cb ? sub { shift; $self->$cb(@_) } : ());
return $self->rendered;
}

sub write_chunk {
my ($self, $chunk, $cb) = @_;
($cb, $chunk) = ($chunk, undef) if ref $chunk eq 'CODE';
my $content = $self->res->content;
$content->write_chunk($chunk, $cb ? sub { shift; $self->$cb(@_) } : ());
return $self->rendered;
Expand Down Expand Up @@ -944,7 +942,6 @@ excessively large, there's a 16MB limit by default.
$c = $c->write;
$c = $c->write('');
$c = $c->write($bytes);
$c = $c->write(sub {...});
$c = $c->write($bytes => sub {...});
Write dynamic content non-blocking, the optional drain callback will be invoked
Expand Down Expand Up @@ -998,7 +995,6 @@ defaults to C<15> seconds.
$c = $c->write_chunk;
$c = $c->write_chunk('');
$c = $c->write_chunk($bytes);
$c = $c->write_chunk(sub {...});
$c = $c->write_chunk($bytes => sub {...});
Write dynamic content non-blocking with chunked transfer encoding, the optional
Expand Down
5 changes: 4 additions & 1 deletion t/mojo/websocket.t
Expand Up @@ -138,7 +138,7 @@ $ua->websocket(
}
);
Mojo::IOLoop->start;
Mojo::IOLoop->one_tick until exists $stash->{finished};
Mojo::IOLoop->one_tick until $stash->{finished};
is $stash->{finished}, 1, 'finish event has been emitted once';
like $result, qr!test1test2ws://127\.0\.0\.1:\d+/!, 'right result';

Expand Down Expand Up @@ -199,6 +199,7 @@ $ua->websocket(
}
);
Mojo::IOLoop->start;
Mojo::IOLoop->one_tick until $stash->{finished};
is $stash->{handshake}, 1, 'finish event has been emitted once for handshake';
is $stash->{finished}, 1, 'finish event has been emitted once';
ok !$ws, 'not a WebSocket';
Expand All @@ -216,6 +217,7 @@ $ua->websocket(
}
);
Mojo::IOLoop->start;
Mojo::IOLoop->one_tick until $stash->{finished};
is $stash->{finished}, 1, 'finish event has been emitted once';
is $code, 101, 'right status';
is $result, 'test0test1', 'right result';
Expand Down Expand Up @@ -385,6 +387,7 @@ $ua->websocket(
}
);
Mojo::IOLoop->start;
Mojo::IOLoop->one_tick until $stash->{finished};
is $stash->{finished}, 1, 'finish event has been emitted once';
like $log, qr/Inactivity timeout/, 'right log message';
app->log->unsubscribe(message => $msg);
Expand Down
4 changes: 3 additions & 1 deletion t/mojolicious/app.t
Expand Up @@ -14,6 +14,7 @@ use lib "$FindBin::Bin/lib";
use File::Spec::Functions 'catdir';
use Mojo::Asset::File;
use Mojo::Date;
use Mojo::IOLoop;
use Mojolicious;
use Test::Mojo;

Expand Down Expand Up @@ -572,7 +573,8 @@ my $stash;
$t->app->plugins->once(before_dispatch => sub { $stash = shift->stash });
$t->get_ok('/longpoll')->status_is(200)
->header_is(Server => 'Mojolicious (Perl)')->content_is('Poll!');
ok $stash->{finished}, 'finish event has been emitted';
Mojo::IOLoop->one_tick until $stash->{finished};
is $stash->{finished}, 1, 'finish event has been emitted once';
ok $stash->{destroyed}, 'controller has been destroyed';

# MojoliciousTest::Foo::config
Expand Down
4 changes: 2 additions & 2 deletions t/mojolicious/lib/MojoliciousTest/Foo.pm
Expand Up @@ -20,8 +20,8 @@ sub longpoll {
my $self = shift;
$self->on(finish => sub { shift->stash->{finished} = 1 });
$self->write_chunk(
sub {
shift->write_chunk('Poll!' => sub { shift->write_chunk('') });
'P' => sub {
shift->write_chunk('oll!' => sub { shift->write_chunk('') });
}
);
}
Expand Down
7 changes: 4 additions & 3 deletions t/mojolicious/lite_app.t
Expand Up @@ -181,8 +181,8 @@ get '/stream' => sub {

get '/finished' => sub {
my $c = shift;
$c->on(finish => sub { shift->stash->{finished} *= 2 });
$c->stash->{finished} = 1;
$c->on(finish => sub { shift->stash->{finished} -= 1 });
$c->stash->{finished} = 2;
$c->render(text => 'so far so good!');
};

Expand Down Expand Up @@ -667,7 +667,8 @@ my $stash;
$t->app->plugins->once(before_dispatch => sub { $stash = shift->stash });
$t->get_ok('/finished')->status_is(200)
->header_is(Server => 'Mojolicious (Perl)')->content_is('so far so good!');
is $stash->{finished}, 2, 'finish event has been emitted once';
Mojo::IOLoop->one_tick until $stash->{finished};
is $stash->{finished}, 1, 'finish event has been emitted once';

# IRI
$t->get_ok('/привет/мир')->status_is(200)
Expand Down

0 comments on commit cce3b85

Please sign in to comment.