in reply to Re^2: yet another thread question: is Symbol::gensym threadsafe?
in thread yet another thread question: is Symbol::gensym threadsafe?

The first thing I notice is that you are assuming that when you do $sock->recv( $buf, 10 ), that provided the call doesn't return undef, then you will have received the number of bytes requested. I think that this is mistaken and that recv will fetch as many bytes as are available up to the number requested, but it may be less. For your short command packets this probably won't occur, but when you are sending large blocks of serialised content, even though you are sending it as a single chunk, the tcp protocol is free to break it up into a number of packets for transmission. That means that your code may be receiving partial transmissions.

You aren't going to want to hear this, but all in all, your 'protocol' handling seems to be very sloppy, leaving numerous windows of opportunity for failure. At the very least you need to have your recv code check the number of bytes received and loop back for more if it didn't get enough. You should probably also be checking the number of bytes sent, though that's generally less of a concern.

Possibly the best solution would be to ditch your (from your own comments:) "primitive protocol", and switch to using a known good implementation of a well tested protocol. From what I can see, HTTP GET and the lesser known PUT would meet most of your requirements.

Whilst it may seem like a lot of work, I think switching your code to use HTTP::Daemon with it's long pedigree would probably be easier than trying to fix up your home-brew protocol implementation to cover all the possibilities and caveats that HTTP has already got covered and Gisle Aas has already implemented.

You might also consider something like HTTP::Daemon::Threaded, though that is much less well proven and so something of an unknown quantity.

Also, from my first glance at your _merge() routine, it looks to me that it is, at the very best, doing things the hard way in terms of walking the parallel structures to discover differences. I think it could be made considerably simpler. And faster. I also think I see possibilities for corruption, but I need to sit down and go through it again when I've got nothing else on the go. I'll get back to you on that.

Sorry that this is rather negative, but you really do need to fix your protocol handling, and sooner rather than later.


Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
"Too many [] have been sedated by an oppressive environment of political correctness and risk aversion."

Replies are listed 'Best First'.
Re^4: yet another thread question: is Symbol::gensym threadsafe?
by TOD (Friar) on Oct 25, 2007 at 05:26 UTC
    you are definitely right. this Session::... stuff is rather ancient code and has never been changed since, and the reason why it has always worked before might simply be that session objects don't become large. but planets and fleets data do, and i'm quite sure that's where the difficulties came from. i modified the respective parts as follows:
    sub receive { my $self = shift; # Perl OO mantra + ... return undef unless ref $self; # .. continued unless (@_) { $self->{ERROR}->set(ILL_NUM); return 0; } my $sock = shift; local $/ = undef; unless (defined $sock->recv($self->{COMMAND}, $self->{COMMANDSIZE}) + && defined $sock->recv($self->{CONTENT_LENGTH}, $self->{CONTEN +TSIZE})) { $self->{ERROR}->set(SOCK_RECV); return 0; } my $cont = ''; $self->{CONTENT} = ''; while (length $self->{CONTENT} < $self->{CONTENT_LENGTH}) { unless (defined $sock->recv($cont, $self->{CONTENT_LENGTH})) { $self->{ERROR}->set(SOCK_RECV); return 0; } $self->{CONTENT} .= $cont; $cont = ''; } 1; }
    and, although the loop doesn't seem to be necessary:
    sub send { my $self = shift; return undef unless ref $self; my $sock = shift || $self{ERROR}->set(ILL_NUM); return 0 if $self->error->get; $self->{CONTENT_LENGTH} = length $self->{CONTENT}; my $str = sprintf("%$self->{CODESIZE}s", $self->{CODE}) . sprintf("%$self->{CONTENTSIZE}d", $self->{CONTENT_LENGTH} +) . $self->{CONTENT}; my $length = length $str; local $\ = undef; my ($sent, $tsend) = (0, 0); while ($sent < $length) { unless ($tsent = $sock->send($str)) { $self->error->set(SOCK_SEND, $!); return 0; } $sent += $tsent; $tsent = 0; } 1; }
    i'm quite sure that this will fix the problem, and once again you nearly saved my life. :)
    --------------------------------
    masses are the opiate for religion.