in reply to Re^2: Problem with using threads with modules.
in thread Problem with using threads with modules.

There are a couple of problems I see with what you are doing.

  1. Your Update() routine splits the contents of $var into lines and then prints them to STDOUT but never clears that variable.

    This means that each time Update() loops, it will be re-processing all the data it has previously processed along with any new data that has been added.

  2. Your starting a new thread running Update() for each run() thread you are creating.

    Each of these threads is accessing the same copy of $var and outputting to the same screen. Each copy of Update() will therefore be processing (and re-processing) the same data--and repetitiously outputting it to the same screen.

If, as is suggested by your code, you simply want all the logging output from all your Telnet sessions to be logged to STDOUT, why not just do $sock_>input_log( \*STDOUT );?

That way, the module will log directly to STDOUT and you don't need all the extra threads or code.

If you need to do some pre-processing of the logged data prior to output, then your use of IO::String may make some sense, but you still don't need one Update() thread per run thread. Just a single Update() thread is sufficient. You would still need to correct problem 1 above.

That said, if you are using 5.8.x for this, and you should be as threads were less than complete prior to 5.8.3, then it may well be better to use the "in-memory files" ability of Perl's open to avoid the need for this module at all. (Note: I haven't tried this in conjunction with threads yet!)

I would (probably) use the original (main) thread for this purpose though as your code implies that this is all embedded in a module, it is unclear to me how the module would be used, so I reserve judgment in that.

Finally, you are still loading Net::Telnet with use. As previously explained, this means that every thread in your application will load a copy of it, including those that have no need for it.

If you reduce the number of Update() threads to one, and your application doesn't create any other threads besides the run threads and the one Update thread, this probably wouldn't make a big difference to the size of your app. But as coded, half of all your threads will carry a copy of Net::Telnet but never make use if it.

As always, with a clearer understanding of what the overall aim of your app is, it would be easier to make suggestions.


Examine what is said, not who speaks.
"Efficiency is intelligent laziness." -David Dunham
"Think for yourself!" - Abigail
"Memory, processor, disk in that order on the hardware side. Algorithm, algoritm, algorithm on the code side." - tachyon

Replies are listed 'Best First'.
Re^4: Problem with using threads with modules.
by tele2mag (Initiate) on Jun 23, 2004 at 21:41 UTC

    Background: To make you understand why I did as I did, a small explanation of the surrounding application is required.
    As you correctly pointed out, this is a module for a bigger application that uses Telnet to extract information and execute commands on some hardware devices (in this case Routers). This application is not timecritical due to it's usage, but some optimization I am planning.

    Main app: The main application has a GUI using Tk module. The main thread handles all user input, while all the other threads handles Telnet sessions that is connected to hardware devices. Every thread has its presentation in its own window in main GUI (not always visible though). So the application helps the user to command several devices at the same time.

    You might think, why not using fork? Well I thought threads would work fine and had more potentials.
    Anyway, all logging from the devices is presented in a textwidget. The Telnet-threads should also notify the user if timeouts occur and by their progress. Thats isn't implemented yet. Tk and threads does not go well together so I am going to supply the threads with callback-references to update methods in my GUI app.
    Now to answering a few questions you had.

    The Update() method is not my final version, just a quick one I wrote to test my app. It had to deliver the Telnet-communication log (stored in $var)in a line-by-line style so it looks like the output you'd get on a terminal if you manually did this. As I mentioned above, it delivers those lines to a callback method in my GUI. Then the callback function inserts the lines into my textwidget.
    I wrote to STDOUT to be able to test it without the GUI present, so that is not my intension at all.

    You said that I should clear $var, but I can't because I want to save the whole comm-log to file when execution finishes.
    One solution is to extract substrings of $var and use that instead. So you know, it didn't feel right to me either with the re-processing lines. It will have no or little effect on the general performance of the program but I would never have it there anyway.

    About using a single Update() thread sounds good, think I will use that advice. Did not know that open could be used for "in memory files" so I'll go for that too. Using the main thread to use open() was also my intension so that I'm going to do.
    Instead of use as you pointed out, a require in get_connection() is better. I suppose it stays in memory (as you explained earlier) so that I don't need to repeat it in other methods.

    Again, thanks for all the help.

      Rather than messing around with IO::String or in-memory files and then needing to use substr to try and extract the new bits of the log etc., I think I have an alternative that will achieve your aim.

      The basic idea is to tie a filehandle to a Thread::Queue object, which can be shared across threads. You tie a filehandle and then pass this into each of your Telnet threads and they pass it to their respective Net::Telnet objects for logging.

      Your main thread can retrieve the thread::Queue handle from the tie and the loop over reading from it and displaying what it gets into the appropriate window(s). It can also, output the lines to the permeanent log file(s) within the same loop.

      This 'loop' could also be a Tk event callback.

      I hope the following (crude) code will demonstrate the idea.

      #! perl -slw use strict; use threads qw[ yield ]; use Thread::Queue; package Tie::Glob::Queue; sub TIEHANDLE { my $class = shift; my $self = new Thread::Queue; return bless \$self, $class; } sub PRINT { my $Q = ${ shift() }; $Q->enqueue( join( $, || '', @_ ) ); } 1; package main; our $TELNETS ||= 10; my $threads : shared = 0; sub telnets{ { lock $threads; $threads++ } my $io = shift; for( 1 .. 5 ) { print $io threads->tid(), " : $_"; select undef, undef, undef, rand 1; } { lock $threads; $threads--; } } my $Q = ${ tie *LOG, 'Tie::Glob::Queue' }; my @threads = map{ threads->create( \&telnets, \*LOG )->detach } 1 .. +$TELNETS; yield while $threads < $TELNETS; while( $threads ) { if( $Q->pending ) { print $Q->dequeue; } else{ yield; } }

      The tie package will need filling out (probably by inheriting from Tie::StdHandle) and the telnets() sub is just a placeholder but the basic methodology is there.


      Examine what is said, not who speaks.
      "Efficiency is intelligent laziness." -David Dunham
      "Think for yourself!" - Abigail
      "Memory, processor, disk in that order on the hardware side. Algorithm, algoritm, algorithm on the code side." - tachyon

        I have looked over your code during the weekend. I understand the idea but not all the syntax though.
        In our $TELNETS ||= 10;, what does " || " do?
        And this: select undef, undef, undef, rand 1; ?
        In $Q->enqueue( join( $, || '', @_ ) );, $ stands for the joining character and @_ is the arguments. But what is " || " and " '' " doing there?
        I'm not lacy, just couldn't find that type of syntax in my Perl litterature. I have Perl CD bookshelf (E-book) and Perl-power (hardcopy) amongst other.

        Other than that, it looks good. I have to inherit a skeleton class IO::Handle but leave the major methods blank. Hope telnet-socket only require writing to handle or this whole thing will not work easilly.
        It will not work as good if telnet-socket is writing in char-by-char fashion instead of line-by-line. I can check that out later.

        This is the behaviour I had in mind for the code your supplied:
        I use the main (GUI) thread to create a tie to the thread::queue as you have done.
        Main-thread starts a new telnet-thread and passes that tied queue. Telnet-thread creates a socket with logging to tied-filehandle. Then command-execution to the telnet-socket starts (and later ends) while logging occur.
        Main-thread then starts a Update-thread that uses the tied-handle to save logging to file and calls a callback function that output the logging to the right GUI window.
        For that to work, all the telnet threads must print tagged-messages to tied-handle so I can separate them in my Update()-thread and output them to the right file and window.

        So my program will have.

        • a Main-thread that handles GUI and user input
        • a Update-thread that updates GUI-window(s) and file(s) with telnet-traffic.
        • 1..n, Telnet-threads that connects and executes commands on various hardware devices.

        Main-thread may start another telnet-thread when user gives some appropriate GUI command. (So they don't start at the same time). The number of telnet-threads is limited by the number of hardware access points.

        This is not so different to your behaviour description and it looks promising. By the way, I got the message "A thread exited while 2 threads were running." while I ran your code. It was not an error so I was not to bad.
        Thanks, for redirecting me. Hope it works out for me now :)