Problem with Threaded Socket Server

[piece]

Hi monks, i have a arduino board here which is connected via usb to my linux server.
On the linux server i have written a server / daemon in perl and a client also in perl, which communicates with the perl server via tcp sockets.

The perl server creates a new thread for every incoming connection.

And here is my problem: If i detach the threads or join them, when they are done it crashes my serial connection to the arduino board. If i keep the threads alive doing nothing, everything works, but my perl server generates constantly threads and never cleans them up. What am i doing wrong?

Here is some code. Hope i cover the main section. The whole code is lengthy. I can post all if needed.

Perl server:

#! /usr/bin/perl -w

use threads;
use threads::shared;
use IO::Socket;
use Device::SerialPort;
use DateTime;

$| = 1;

my $listen = IO::Socket::INET->new(
        ReuseAddr => 1,
        Reuse    => 1,
        Listen => SOMAXCONN,
        Proto     => 'tcp',
        LocalAddr => 'localhost:9927',
        Timeout => 1
    ) or die "Failed to create socket!\n";

our $usb_lock : shared;

our $arduino = undef;

sub getArduinoConnection {
    #my $oldport = shift;
    #$arduino->close() if defined;
    my $arduino = Device::SerialPort->new("/dev/serial/by-id/usb-Ardui
+no__www.arduino.cc__Arduino_Mega_2560_74136333033351011152-if00");
    if ( defined( $arduino ) ) {
        $arduino->baudrate(9600);
        $arduino->parity("none");
        $arduino->databits(8);
        $arduino->stopbits(1);

        $arduino->read_char_time(0);
        $arduino->read_const_time(500);
        $arduino->write_settings || undef $arduino;
        writelog( "server:[portSet]" );
    } else {
        writelog( "server:[portSet]: failed" );
    }
    return $arduino;
}

....
    
sub handle_connection {
    #sleep( 1 );
    my $socket = shift;
    my $local_arduino = shift;
    my $clientport = $socket->peerport();
    $socket->autoflush(1);
    while ( my $cmd = <$socket> ) {
        # get full command or die
        my $count = 0;
        while ( !cmd_complete($cmd) ) {
            $cmd .= <$socket>;
            $count++;
            if ( $count > 20 ) { 
                writelog( "server:" . $clientport . "[cmdInc]: no cmd 
+end" );
                return 0;
            }
        }
        chomp($cmd);
        writelog( "server:" . $clientport . "[cmdInc]: " . decode_cmd(
+ $cmd ) );
        {
            # send command to arduino
            lock( $usb_lock );
            writelog( "server:" . $clientport . "[lockusb]" );
            $local_arduino->write($cmd);
            
            # get arduinos answer
            my $answer = ".";
            while ( $answer ne "" ) {
                $answer = getLineFromArduino();
                writelog( "server:" . $clientport . "[cmdAnswer]: $ans
+wer" );
                #print $socket "$answer\n";
                if ( $answer eq "Command end" ) {
                    last;
                }
            }
            # respond to client
            if ( $answer eq "Command end" ) {
                writelog( "server:" . $clientport . "[sendToClient]: \
+"ok\"" );
                print $socket "ok\n";
            } else {
                writelog( "server:" . $clientport . "[sendToClient]: \
+"fail\"" );
                print $socket "fail\n";
            }
            writelog( "server:" . $clientport . "[unlockusb]" );
        }
        return 1;
    }
}

my $last_validation = -10; #in the past
#main loop
while ( 1 ) {
    foreach $thr (threads->list) { 
        # Don't join the main thread or ourselves 
        #print $thr->tid;
        if ($thr->tid && $thr->is_joinable() ) { #&& !threads::equal($
+thr, threads->self)
            #$thr->join; 
        } 
    }
    # validate arduino every 10 seconds
    # sleep if arduino not alive
    if ( time > ( $last_validation + 30 ) ) {
        lock( $usb_lock );
        $last_validation = time;
        $arduino = validateArduinoConnection( $arduino );
        until ( defined( $arduino ) ) {
            $arduino = validateArduinoConnection( $arduino );
            sleep( 1 );
        }
        clearUSBdata();
    }
    if (my $socket = $listen->accept) {
        #async(\&handle_connection, $socket);
        threads->create(\&handle_connection, $socket, $arduino );
    }
}

Perl client:

#! /usr/bin/perl -w

use threads;
use IO::Socket;
use DateTime;

my $socket = IO::Socket::INET->new(
        Proto   => 'tcp',
        PeerAddr=> 'localhost',
        PeerPort=> "9927",
        Reuse   => 1,
        Timeout => 1
    ) or die "ERROR in Socket Creation : $!\n";
    
my $select = IO::Select->new($socket) or die "IO::Select $!";
my $localport = $socket->sockport();

sub writelog {
    my $dt   = DateTime->now;
    my $date = $dt->ymd . " " . $dt->hms;
    my ( $line ) = @_;
    open (LOGFILE, '>>/var/log/home/roll.log');
    print LOGFILE "$date - $line\n";
    close (LOGFILE);
}

sub decode_cmd {
    my $cmd = shift;
    my @str = split(//, $cmd);
    my @cmds = ();
    my $cmd_str = "";
    for ( my $i = 0; $i < @str; $i++ ) {
        push( @cmds, ord( $str[ $i ] ) );
    }
    return join( ',', @cmds );
}

#main

# translate argument int to chr and build command string
my $num_args = $#ARGV + 1;
my $cmd = "";
for ( my $i = 0; $i < $num_args; $i++ ) {
    if ( $ARGV[$i] < 256 ) {
        $cmd .= chr($ARGV[$i]);
    } else {
        $cmd .= pack("n",$ARGV[$i]);
    }
}
$cmd = chr(254) . $cmd . chr(255); # 254=cmd begin 255=cmd stop

writelog( "client:" . $localport . "[sendCmdToServer]: " . decode_cmd(
+ $cmd ) );
# send command to server
print $socket "$cmd\n";

# wait for servers answer
my $line = "";
if ( $select->can_read(5.25) ) {
    chomp( $line = <$socket> );
}
writelog( "client:" . $localport . "[serverAnswer]: \"$line\"" );

Any help greatly appreciated.

[BrowserUk]

If i detach the threads or join them, when they are done it crashes my serial connection to the arduino board.

You do not close the connection to your USB device before returning from your thread proc, so the symptoms above lead me to the conclusion that when Perl tries to garbage collect your thread procs -- ie. when you either join them; or immediately when you return if you detach them; it tries to DESTROY the local copy of the usb device handle $local_arduino. And that closes your connection.

You can confirm this speculation if you enable DEBUG on the device handle and you see a debug message of the form:

"Destroying $self->{NAME}" at ... etc

You might be able to work around Device::SerialPort's lack of thread-awareness by overriding its DESTROY method:

sub DESTROY {
    my $self = shift;
    return unless (defined $self->{NAME});
    if ($self->{"_DEBUG"}) {
        carp "Destroying $self->{NAME}";
    }
    $self->close;
}

And preventing it from closing the connection unless it is the last instance of the handle; or perhaps, only if the DESTROY method is being called from the same thread as created the connection. How you would arrange to know either of these things is down to your ingenuity.

Alternatively, and more likely to work, would be to have a single thread only communicate with your USB device.

In this scenario, you start a thread which creates the connection to the USB device; and pass it a queue handle at start up. That thread the loops reading that Q waiting for command requests from other threads; enacts them; and then returns the results to the requesting threads (via a queue handle they pass along with the command request).

Your USB handling thread would look something like this:

sub USB {
    my $Q = shift;
    my $arduino = Device::SerialPort->new("/dev/serial/by-id/usb-Ardui
+no__www.arduino.cc__Arduino_Mega_2560_74136333033351011152-if00");
    if ( defined( $arduino ) ) {
        $arduino->baudrate(9600);
        $arduino->parity("none");
        $arduino->databits(8);
        $arduino->stopbits(1);
        $arduino->read_char_time(0);
        $arduino->read_const_time(500);
        $arduino->write_settings || undef $arduino;
        writelog( "server:[portSet]" );
    } else {
        writelog( "server:[portSet]: failed" );
    }
    while( my $request = $Q->dequeue ) {
        my $retQ = $Q->dequeue;
        $arduino->write($cmd);
        my $answer = getLineFromArduino()
        $retQ->enqueue( $answer );
    }
}

To make requests, your client threads then do:

sub client {
    my $client = shift;
    my $usbQ = shift;
 
    my $retQ = new Thread::Queue;
    ...

    my $command = <$client>;
    $usbQ->enqueue( $command, $retQ );
    my $result = $retQ->dequeue;
    print $client $result;
    ...
}

Season to taste with logging and error handling.

[piece]

Just like you assumed:



Destroying /dev/serial/by-id/usb-Arduino__www.arduino.cc__Arduino_Mega
+_2560_74136333033351011152-if00 at ./roll_server.pl line 0 thread 1


I will try to implement your suggestions. Though it could take a while with my perl knowledge :)

Is it somehow possible to declare the $arduino object as shared, so the garba collection avoids it?
This

my $arduino : shared;

does not work.

So the idea is to create a new Device::SerialPort class which inherits the original class and overrides the destructor, right?

[BrowserUk]

Is it somehow possible to declare the $arduino object as shared, so the garba collection avoids it?

Not directly no. Each thread gets its own copy (actually not a fully copy, but a proxy to the real thing) of each shared variable; and that proxy is local to the thread and must be destroyed before the thread ends. In many cases, the appropriate action is to do nothing in the destroy unless this is the last copy of the handle.

In theory, there is a mechanism whereby module authors can make their modules thread-aware and possible thread-safe, by taking control of the cloning process by defining sub CLONE{ .. } in their packages; but the mechanism is barely described and I've never seen an example of anyone using it; so I cannot comment further on it. Besides which you'd either have to get the author of D::SP interested; or make your own changes.

[BrowserUk]

Just saw your update:

So the idea is to create a new Device::SerialPort class which inherits the original class and overrides the destructor, right?

That is the essence of the first of the two possibilities I offered. And you could go the full subclassing route if that's your thing. Personally, I wouldn't.

I'd just inject my override directly into the D::SP namespace. Ie. At the top of your program somewhere after you've used Device::SerialPort, define a subroutine:

sub Device::SerialPort::DESTROY {
    my $self = shift;
    return unless (defined $self->{NAME});
    if ($self->{"_DEBUG"}) {
        carp "Destroying $self->{NAME}";
    }
    #$self->close;
}

You'll get a "subroutine redefined" warning -- which you can disable -- but the affect will be the same as 'doing it properly' without the hassle.

Simply commenting out the #$self->close; may be enough. If not, then you will either have to investigate why not; or try my second suggestion.

Don't forget to call close() in the same thread as you created the USB connection before finishing.

[mhearse]

http://poe.perl.org/?POE_Cookbook/TCP_Servers

[piece]

Do i miss something if the link? I don't see any references to serial ports. My tcp connection between the perl client and server works perfectly with the threads. The problem is the serial/usb connection between the perl server and the arduino board.

[sundialsvc4]

Adding one more thought (in case I missed it) to BrowserUK’s excellent recommendations here ... (++)x2 ... it is also a very good idea to create a pool of worker-threads whose sole purpose is to wait for an incoming message on a shared queue that they all read, and to carry out that request.   (Usually, having done so, they write a record to an outbound-queue so that it can be sent back to the client, either by the “reader” thread or by another one.)

The number of threads is not-equal to the number of requests that are in process at one time, and so the system never attempts to bite off more than it can chew ... the requests just have to wait in line for a bit, but they do so cheaply.   The overhead of setting-up and tearing-down a process or thread is also greatly reduced.   You can set your watch by how many requests-per-second such a system will be able to churn through, no matter whether its waiting-room is full or empty.