in reply to Containing threads in object instances

Whilst it is right to say that you cannot use objects across threads, what this means is that you cannot call methods on an object created in one thread from another thread. For the purposes of what you are trying to do, you don't need to do that, and your basic idea of trying to do an asynchronous write with timeout is perfectly feasible to with iThreads.

However, there are several errors in your implementation.

The first thing to clarify is the concept of calling an instance method versus a class method. There is no difference between an instance method and a class method in perl. (Actually, for the most part this is true for all OO, but there is a possible debate in there that I would rather avoid, so pretend I didn't add that last bit:)

All methods are just subs. The only difference between calling them as a class method and an instance method, is that when you invoke the sub as an instance method, perl supplies the handle of the object through which you invoked the sub as the first parameter to the call. Ie. it unshifts the object handle onto @_. So the answer to the problem of how to pass an instance method address to the threads->create() call is to take the address of the class method and explicitly pass the object handle as the first parameter

my $childThread = threads->new( \&Your::Package::childThreadWrite, $se +lf, $l_cqrdMessage);

However, this will not work with your code as is. There are several reasons

  1. The hashref that is the object handle for your instance is not shared.

    As soon as you get into childThreadWriter and attempt to execute

    lock $self->{waitFlag};

    you will receive the message lock can only be used on shared values at ...

    Your first though might be to try and share $self, but you will receive the error

     Invalid value for shared scalar at ...

    or  Cannot share globs yet at ...

    You could use a "splendid bareword" for your pipe handle, being global they are implicitly shared by all threads in a process, so this will work. The problem is that it will limit you to only one instance of your class :(.

    Now it may be that you only intend to have one external process running at any given time and so a single instance wouldn't be the end of the world, but it goes rather against the grain for an OO module to have this restriction. There is a way around this.

    This is the first time I've ever suggested using symbolic references, and the only situation where I haven't found any other way to achieve the desired goal. By passing in a name when you create an instance, and using this symbolically as the name of your pipe handle, it becomes possible to have your cake and eat it:)

    print do{ no strict 'refs'; \*( $self->{ inst } } }, $l_cqrdMessage;
  2. Once you have a mechanism for passing the handle to the pipe, the next problem is to have a way of timing out the print to the pipe.

    You are attempting to do this using the self->{ waitFlag } element of your instance data. Now that we have removed the GLOB from the object hash, we could share this hash with the writer thread. However, there is a problem with the logic of your flagging mechanism.

    Your logic goes like this.

    1. You set the flag to 0
    2. You start the thread.
    3. You wait in the main thread (yielding) until the flag goes to 1
    4. The child thread starts and sets the flag to 1.
    5. The main thread sees this and exits the first wait loop.
    6. It enters the second where it waits (again yielding) for the flag to return to 0 (or timeout).
    7. The child thread prints to the pipe. We'll assume successfully.
    8. It sets the flag to 0.
    9. The main thread detects the change, sees the flag changed rather than timeout occurred and returns to the caller.

    The problem is that threads are not deterministic. Consider what happens when the main thread yields at step 3. The child thread probably gets to run. It sets the flag to 1, prints to the pipe, and set the flag back to 0 again and terminates. It is only once that thread terminates that the main thread will get to run again. However, it is still sitting waiting for the flag to go to 1, which it will never do because the only thing that would cause it to change is the thread, which has already been through the complete cycle and finished. Your main thread is now blocked forever waiting for the flag to change.

    We all tend to think that threads run simultaneously, even though we known deep down that there is only one CPU, and only one thread can be using it at a time:).

    The (or rather a) correct way to do the handshaking is

    Main thread.

    1. Set the flag =1 in the main thread.
    2. Start the thread.
    3. Wait in the main thread until the flag goes to 0 (or timeout occurs).
    4. Check whether the loop exited because the flag changed or timeout and take the appropriate action.

    Child thread.

    1. Print to the pipe.
    2. Clear the flag = 0.
    3. Terminate.

    In this way, no deadlock can occur because the main thread is only waiting for one action by the child thread (or timeout) rather than two transitions.

  3. You are storing the flag and timeout values as instance data.

    This is unnecessary as these values are used wholly within the context of the write method. I presume that you did this as an easy way of passing them to the childThreadWriter code, but actually it creates more problems than it solves. To this end, I have used shared lexicals in the write method and passed them to the thread as parameters, which made life a little easier.

Here is some (incomplete, especially error checking) code that demos the suggestions above.

#! perl -slw use strict; package test; use threads qw[ yield ]; use threads::shared; use IPC::Open2; use constant TIMEOUT => 10; # seconds sub new { my( $class, $inst ) = @_; my $pid = open2( '>&STDOUT', do{ no strict 'refs'; \*{ $inst } }, 'perl58', '-e" print q[etx:], scalar <> for 1 .. 5; sleep 60; + print q[Dying]; "' ) or die "Opening pipe failed: $!"; my %self : shared = ( inst => $inst, procID => $pid, ); return bless \%self, shift; } sub write { my( $self, $msg ) = @_; my $flag : shared = 1; warn 'Setting timeout'; my $timeout = time + TIMEOUT; my $thread = threads->create( \&tWrite, $self, \$flag, do{ no strict 'refs'; \*{ $self->{ inst } } }, $msg ); warn 'Waiting for flg to clear or timeout'; yield while do{ lock( $flag ); $flag } and time < $timeout; if( $flag ) { warn "Timeout; killing process:$self->{ procID }"; kill 9, $self->{ procID } or warn $!; warn 'Closing pipe'; close do{ no strict 'refs'; \*{ $self->{ inst } } } or warn "Close pipe failed: $!"; @{ $self }{ qw[ procID timeout ] } = ( undef, 0 ); warn 'Waiting for thread to notice'; $thread->join; warn 'Returning failure'; return 0; } warn 'Wrote successfully; waiting for thread to join'; $thread->join; warn 'joined'; return 1; } sub tWrite { my( $self, $flagref, $GLOB, $msg ) = @_; warn 'Writing'; print { $GLOB } $msg or warn "pipe: $!"; warn 'Written; clearing flag'; { lock( $$flagref ); $$flagref = 0; } warn 'Ending'; } sub done { my( $self ) = @_; close do{ no strict 'refs'; \*{ $self->{ inst } } } or warn $!; kill 9, $self->{ ProcID }; undef $self; } 1; package main; select do{ local $_ = select( STDERR ); $|++; $_ }; # Pass in a name to be used symbolically # for this instance pipe handle. my $test = test->new( 'PIPE' ); for (1 .. 10) { printf 'Enter to print a line of data'; <STDIN>; warn 'Calling write'; unless( $test->write( 'X' x 1024 ) ) { warn 'Restarting external process'; $test->done; # Use the same name when we recreate the instance. $test = test->new( 'PIPE' ); } else { warn "Successfully wrote line $_"; } } warn 'Done'; exit;

I've left all the tracing and a crude single step in place as the combination allows you to watch the process go by. To simulate the failing external process I've spawned a perl one-liner that reads 3 lines and then goes to sleep. The effect is that when you hit enter for the 6th time, the one-liner will take no action to process it, and so the timeout kicks in and you get to watch the external process be killed and recreated, at which point you can go one to print another 5 lines before the program terminates.

One last thing, under win32, there is very little difference between spawning a thread and forking a process, as forks are implemented using threads under the covers. If anything, the need to be able to share data may actually mean that spawning a thread may be slightly more costly. However, if you need to share data, it is a lot easier than setting up IPC for yourself:)

For most purposes, I would suggest not spawning a thread each time you want to do something asynchronously, but rather start a thread and having it sit clocked in the background until you give it something to do. Have it wake up, do it and go back to sleep again till you need it again. This is usually considerably more efficient.

In this case, that way of operating doesn't fit the requirement. Or at least I can't yet see how to make it fit the requirements:)

I learnt a good deal following this through...HTH


Examine what is said, not who speaks.
"Efficiency is intelligent laziness." -David Dunham
"When I'm working on a problem, I never think about beauty. I think only how to solve the problem. But when I have finished, if the solution is not beautiful, I know it is wrong." -Richard Buckminster Fuller

Replies are listed 'Best First'.
Re: Re: Containing threads in object instances (it can be done) (long post).
by liz (Monsignor) on Jul 29, 2003 at 08:21 UTC
    ...For most purposes, I would suggest not spawning a thread each time you want to do something asynchronously, but rather start a thread and having it sit clocked in the background until you give it something to do...

    In that respect you might want to have a look at Thread::Pool.

    Liz

Re: Re: Containing threads in object instances (it can be done) (long post).
by tid (Beadle) on Jul 31, 2003 at 05:08 UTC

    Hey thanks for that. I'll have to work my way through your code example sometime shortly, but as per usual there are other fires I have to put out short term.

    I had found the potential deadlock while waiting for some responses, but rather than confuse the current issue I decided to let it be. That's my story, and I'm sticking to it :)

    The main reason I went with the concept of an object was to be able to instantiate an object that would encapsulate all this potential blocking from a test script. As the test requires approximately 30 different potentially blocking processes of the same type running simultaneously, your OO instincts about limiting the usage of the class to one object were entirely correct. I'm a contractor first and purist second, so I wouldn't have invested the time to learn about Perl Objects on someone elses time (and money) unless the task at hand required it.

    Hopefully I've learned as much as you did. Many thanks for the time spent.
    Mike.