Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine
 
PerlMonks  

Re^4: ithreads memory leak

by DNAb (Novice)
on Apr 09, 2015 at 17:47 UTC ( #1122977=note: print w/replies, xml ) Need Help??


in reply to Re^3: ithreads memory leak
in thread ithreads memory leak

Ah I see this caveat noted in the thread docs now, thank you for clearing this up.

I'm happy to listen to any suggestions you can offer (when you have free time of course!), and I'm willing to put work into this, assuming it's near my level of competence.

Again, thanks!

Replies are listed 'Best First'.
Re^5: ithreads memory leak
by BrowserUk (Pope) on Apr 09, 2015 at 19:00 UTC

    I said 3 suggestion, but one of them was too complicated to merit discussion.

    1. So first the "right way" to do it.

      Your walktimer thread sub becomes something like this:

      sub walktimer { ## The queue handle is passed in when the thread is started my $Q = shift; #Sleep for 180 seconds, upon completion execute SQL to change OK_O +PEN back to all zero for $showcasenum ## A hash to remember when things time out. my %when; ## wake up once per second while( sleep 1 ) { ## while there are showcase numbers on the queue while( $Q->pending ) { ## grab the one at a time my $showcasenum = $Q->dequeue(); ## Calculate when the timeout should occur, ## and add the showcase number to the hash to be dealt wit +h at that time push @{ $when{ time() + $walktime } }, $showcasenumber; + } ## find any times that have expired (earlier than now!) for my $time ( grep{ $_ < time() } keys %when ) { ## And for each showcase number scheduled at that time for my showcasenum ( @{ $when{ $time } } ) { ## Do your db stuff my $dbh = DBI->connect('dbi:mysql:alarm:databasehost', +'username','password') or die "Connection Error: $DBI::errstr\n"; my $sql = "update Showcases set OK_OPEN=0 WHERE SC_NUM + = ?"; my $sth = $dbh->prepare($sql) or die "Can't prepare $s +ql: $dbh->errstr\n"; $sth->execute($showcasenum) or die "SQL Error: $DBI::e +rrstr\n"; $dbh->disconnect; } ## and remove the expired time from the hash delete $when{ $time }; } } }

      I hope the comments make it clear, but basically, instead of starting a new thread for each timeout, you start one thread and pass it a handle to a queue.

      Then when you would have started a new walktimer thread, you simply post the showcase number to that queue. Ie this:

      elsif( $okopen == 1) { #Kill the walk timer if it is running, someone opened +the showcase legit debug("Killing walking timer and starting open timer, +authorized user opened showcase $showcasenum"); if( $walkthread[$showcasenum] && $walkthread[$showcase +num]->is_running() && !$walkthread[$showcasenum]->is_detached) { $walkthread[$showcasenum]->kill('KILL')->detach(); } $openthread[$showcasenum] = threads->create(\&opentime +r, $showcasenum, $opentime, $showcasedesc); }

      becomes this:

      elsif( $okopen == 1) { $Qwalktimer->enqueue( $showcasenum ); }

      You do the same thing for your opentimer sub thread.

      Now, instead of starting a new thread each time(r:), and having to track them, and kill them if they still exist; before starting a new one; you:

      1. Create 2 queues:

        One for opentimers and one for walktimers.

      2. Create (and detach) two threads at the start of the program.

        Fire and forget. They'll go away when your main thread does.

      3. You send (queue) instructions to them and they take care of it at the appropriate time.

      Two, long running threads that take instructions and do what's required. No memory growth.

    2. Now, the "quick fix option". Modify your walktimer this way:
      sub walktimer { #Sleep for 180 seconds, upon completion execute SQL to change OK_O +PEN back to all zero for $showcasenum my $showcasenum = $_[0]; debug("Launched walking timer thread for $showcasenum"); $SIG{'KILL'} = sub { threads->exit(); }; my $count = $walktime; sleep( 1 ) while --$count; ### Wake once per second for $walktime +seconds to allow the "signal handler" to respond to any pending "sign +als". if( threads->is_detached()) { threads->exit(); } my $dbh = DBI->connect('dbi:mysql:alarm:databasehost','username',' +password') or die "Connection Error: $DBI::errstr\n"; my $sql = "update Showcases set OK_OPEN=0 WHERE SC_NUM = ?"; my $sth = $dbh->prepare($sql) or die "Can't prepare $sql: $dbh->er +rstr\n"; $sth->execute($showcasenum) or die "SQL Error: $DBI::errstr\n"; $dbh->disconnect; debug("Walking thread for $showcasenum is expiring..."); threads->detach(); }

      Ditto for opentimer(). And that should allow your current architecture to work. (I don't like it but ... its your code :)

    If you choose option 1 and need further help, yell and I'll try to modify a copy of your code and post it, but I'm not going to waste the effort if you choose option 2 :)


    With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
    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". I'm with torvalds on this
    In the absence of evidence, opinion is indistinguishable from prejudice. Agile (and TDD) debunked

      I've looked over both examples and while the second certainly looks easier, I agree that creating and destroying threads constantly is a waste compared to just starting two workers and sending them work.

      I mostly get where you are going with this, but the one thing that I can't visualize is how are we going to disregard the open timer if the user closes the showcase? The work has already been passed to the thread. Also the walk timer needs to respect multiple key presses and update the expiry time, and cease termination and "restart" work.

        I can't visualize is how are we going to disregard the open timer if the user closes the showcase? The work has already been passed to the thread. Also the walk timer needs to respect multiple key presses and update the expiry time, and cease termination and "restart" work.

        Then we need to be able to ask the thread to do two things:

        1. Add (or re-add) a showcase: $Qwalktimer->enqueue( "add:$showcasenum" );
        2. Remove an exist showcase: $Qwalktimer->enqueue( "del:$showcasenum" );

        And that means adding a test : if( exists $showcase{ $showcasenum }  ) { for an showcase number;

        And keeping a record of where the current timer for that showcase is: %showcase.

        Now, when 'add'ing a showcase, we remove any existing timer before adding a new one. And if the cmd eq 'del', then we just remove the existing timer.

        sub walktimer { ## The queue handle is passed in when the thread is started my $Q = shift; #Sleep for 180 seconds, upon completion execute SQL to change OK_O +PEN back to all zero for $showcasenum ## A hash to remember when things time out. my( %when, %showcase ); ## wake up once per second while( sleep 1 ) { ## while there are showcase numbers on the queue while( $Q->pending ) { ## grab them one at a time my( $cmd, $showcasenum ) = split ':', $Q->dequeue(); ## If a timer already exists for this showcase if( exists $showcase{ $showcasenum } ) { my $when = $showcase{ $showcasenum }; ## remove the timer @{ $when{ $when } } = grep{ $_ != $showcasenum } @{ $w +hen{ $when } }; ## and the pointer to it. delete $showcase{ $showcasenum }; ## And if this command is a delete request that's all next if $cmd eq 'del'; } ## Otherwise its a (re)add, so add the new timer ## Calculate when the timeout should occur, my $timeout = time() + $walktime; ## and add the showcase number to the hash to be dealt wit +h at that time push @{ $when{ $timeout } }, $showcasenumber; ## And remember its there $showcase{ $showcasenum } = $timeout; } ## find any times that have expired (earlier than now!) for my $time ( grep{ $_ < time() } keys %when ) { ## And for each showcase number scheduled at that time for my showcasenum ( @{ $when{ $time } } ) { ## Do your db stuff my $dbh = DBI->connect('dbi:mysql:alarm:databasehost', +'username','password') or die "Connection Error: $DBI::errstr\n"; my $sql = "update Showcases set OK_OPEN=0 WHERE SC_NUM + = ?"; my $sth = $dbh->prepare($sql) or die "Can't prepare $s +ql: $dbh->errstr\n"; $sth->execute($showcasenum) or die "SQL Error: $DBI::e +rrstr\n"; $dbh->disconnect; ## remove the pointer for this showcase delete $showcase{ showcasenum }; } ## and remove the expired time from the hash delete $when{ $time }; } } }

        Does that make sense? Does it cover all the bases?


        With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
        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". I'm with torvalds on this
        In the absence of evidence, opinion is indistinguishable from prejudice. Agile (and TDD) debunked

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://1122977]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others chanting in the Monastery: (9)
As of 2019-12-06 16:33 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Strict and warnings: which comes first?



    Results (156 votes). Check out past polls.

    Notices?