Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

starman --listen decided as backlog queue by Net::Server inside #57

Closed
Perlover opened this Issue · 12 comments

2 participants

@Perlover

I believe that the option listen of starman (IP:PORT format for example) is decided by Net::Server inside as backlog option because the last looks in @ARGV as i think

From doc of Net::Server:

listen \d+ SOMAXCONN

I got messages in error.log of starman:

Argument "XX.XX.XX.XX:PPPP" isn't numeric in numeric gt (>) at /usr/local/lib/perl5/5.16.2/x86_64-linux-thread-multi/IO/Socket.pm line 224.

I looked in code, added 'carp' to $SIG{WARN} inside Socket.pm (224) and got:
Argument "XX.XX.XX.XX:PPPP" isn't numeric in numeric gt (>) at /usr/local/lib/perl5/5.16.2/x86_64-linux-thread-multi/IO/Socket.pm line 224.
at /usr/local/lib/perl5/site_perl/5.16.2/Net/Server.pm line 325

The submodule Net::Server::Proto::TCP has lines:

    $sock->NS_listen(defined($info->{'listen'}) ? $info->{'listen'}
                    : defined($server->{'server'}->{'listen'}) ? $server->{'server'}->{'listen'}
                    : Socket::SOMAXCONN());

I am sure that listen there as my IP:PORT

So when i run:

starman --listen IP:PORT --backlog 6000
I have backlog as 'IP:PORT' (may be first digit in my IP address)

Please check up this

Thanks

Perlover

@miyagawa
Owner

I can't reproduce:

➜  ~  starman --backlog 600 --listen 0.0.0.0:8080 dev/Plack/eg/dot-psgi/Hello.psgi
2012/12/11-13:55:54 Starman::Server (type Net::Server::PreFork) starting! pid(25819)
Binding to TCP port 8080 on host 0.0.0.0 with IPv4
Setting gid to "20 20 20 401 12 33 61 79 80 81 98 100 204 402"

Check the versions of Plack, Starman and Net::Server and ugprade to the latest to see if the problem exists.

If it is fixed by upgrading versions, report it to me so that we can fix the dependency if there's any mistake.

@Perlover

Thanks!

I made same tests and result exactly as your one
But if i run a project there i see error again (same machine and modules)
I noticed in code a following:

starman runs Plack::Runner and uses parse_options() for parsing of @ARGV

Plack::Runner::parse_options parses an options but parsed options are not deleted from hash and kept in $self->{argv}

Starman::Server::run kept options in @ARGV again before Net::Server::PreFork::run:
local @ARGV = (@{$options->{argv} || []});

And after runs the $self->SUPER::run (Net::Server::PreFork::run)

I think the 'listen' option is passed to Net::Server::PreFork again through @ARGV and affects there in some situations.
This option means a backlog queue of socket for Net::Server::* and means IP:PORT for Plack/starman
Same name but different values

I don't understand why i cannot reproduce with your example (application Plack/eg/dot-psgi/Hello.psgi) but i see errors there with same Plack/Starman/Net::Server modules for my project.

@Perlover

I made:

vi /usr/local/lib/perl5/site_perl/5.16.2/Starman/Server.pm

Added code:

{
use Data::Dumper;
open FILE, '>', '/tmp/dump.txt';
print FILE Dumper( [ ARGV => \@ARGV, options => $options ] );
close FILE;
}

before $self->SUPER::run and got:

$ less /tmp/dump.txt

$VAR1 = [
          'ARGV',
          [
            '--listen',
            'MY_IP_ADDRESS:MY_PORT',
            '--min_servers=12',
            '--max_servers=48',
            '--min_spare_servers=4',
            '--max_spare_servers=20',
            '--backlog',
            '6000',
            '--pid',
            '/home/my_app/my_app/run/services/starman.pid',
            '--error-log',
            '/home/my_app/my_app/logs/starman/error.log',
            '--access-log',
            '/home/my_app/my_app/logs/starman/access.log',
            '--daemonize',
            '--app',
            '/home/my_app/perl5/bin/my_app.psgi'
          ],
          'options',
          {
            'backlog' => '6000',
            'error_log' => '/home/my_app/my_app/logs/starman/error.log',
            'keepalive_timeout' => 1,
            'min_servers' => '12',
            'port' => 'MY_PORT',
            'host' => 'MY_IP_ADDRESS',
            'socket' => undef,
            'max_servers' => '48',
            'listen' => [
                          'MY_IP_ADDRESS:MY_PORT'
                        ],
            'pid' => '/home/my_app/my_app/run/services/starman.pid',
            'argv' => [
                        '--listen',
                        'MY_IP_ADDRESS:MY_PORT',
                        '--min_servers=12',
                        '--max_servers=48',
                        '--min_spare_servers=4',
                        '--max_spare_servers=20',
                        '--backlog',
                        '6000',
                        '--pid',
                        '/home/my_app/my_app/run/services/starman.pid',
                        '--error-log',
                        '/home/my_app/my_app/logs/starman/error.log',
                        '--access-log',
                        '/home/my_app/my_app/logs/starman/access.log',
                        '--daemonize',
                        '--app',
                        '/home/my_app/perl5/bin/my_app.psgi'
                      ],
            'keepalive' => 1,
            'min_spare_servers' => '4',
            'daemonize' => 1,
            'max_spare_servers' => '20'
          }
        ];
@Perlover

So Net::Server in @ARGV has --listen as IP:PORT but he expects there length of queue (backlog)
So there is bug

@miyagawa
Owner
@Perlover

Net::Server:

sub configure {
my $self = shift;
my $prop = $self->{'server'};
my $template = ($_[0] && ref($_[0])) ? shift : undef;

$self->process_args(\@ARGV, $template) if @ARGV; # command line
$self->process_args($prop->{'_run_args'}, $template) if $prop->{'_run_args'}; # passed to run

if ($prop->{'conf_file'}) {
    $self->process_args($self->_read_conf($prop->{'conf_file'}), $template);
} else {
    my $def = $self->default_values || {};
    $self->process_args($self->_read_conf($def->{'conf_file'}), $template) if $def->{'conf_file'};
}
}
@Perlover
 and how can you reproduce? Can you make a reproducible script that doesn't depend on your application?

I will try it through 2-3 hours

@Perlover

I am trying to reproduce with Plack/eg/dot-psgi/Hello.psgi but i cannot while
But anyway it's bad idea to pass --listen option through @ARGV when in Plack this means IP[:PORT] but in Net::Server it means backlog queue.

Comment from Net::Server:process_args:

# we want subsequent calls to not overwrite or add to previously set values so that command line arguments win

I think better way to remove --listen from @ARGV before Net::Server::PreFork::run method execution

@miyagawa
Owner

But anyway it's bad idea to pass --listen option through @ARGV when in Plack this means IP[:PORT] but in Net::Server it means backlog queue.

Sure, that's why we clear stuff out of @ARGV in Plack::Runner and when evaluating .psgi script etc. but maybe we should do that more aggressively? I can't confirm until you come up with a standalone debug script, because it doesn't occur in my instance.

@Perlover
     Sure, that's why we clear stuff out of @ARGV in Plack::Runner and when evaluating .psgi script etc. but maybe we should do that more aggressively?

Yes, may be Plack cleans @ARGV but Starman restores @ARGV before Net::Server::run execution

I am seeing the code of Net::Server now because i want to understand how to reproduce problem with --listen

@miyagawa
Owner

Yes, may be Plack cleans @ARGV but Starman restores @ARGV before Net::Server::run execution

Right, but that @ARGV is restored from $options->{argv} which is already filtered out from Plack::Runner. plackup common options such as --listen are meant to be filtered out.

...

Ok, now that I tried to confirm the behavior and found that $options->{argv} is being recreated from the original script's ARGV, mainly to support HUP based restart which is not supported right now. See 6abaa02

I will remove this and argv will not propagated to Net::Server.

@miyagawa miyagawa closed this issue from a commit
@miyagawa Undo restoring ARGV since Net::Server could do funky things.
It was meant to be used for HUP based restarts, but that is not
supported right now and this chunk of code is unnecessary. Fix #57
6d7e70d
@miyagawa miyagawa closed this in 6d7e70d
@Perlover

Yes, i confirm that i don't have any warnings from IO::Socket now :)
I installed new version from master branch and now all is OK :)
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.