I wrote some XS modules to make Perl faster and smaller. I'm using this modules in serveral programs, but it doesn't work right.

It seems that Perl doesn't work right with threads.

Hm. You wrote bad XS code and then blame Perl.

That's the question :)

Hm. Then it's a very difficult one to answer--because it not clear from either your description or code exactly what it is you are trying to achieve?

For example, you say:

i need a clean destructor for system resources like file handles,

Why do you need to handle the destruction of these?

And why is your %MY_CXT shared? And global?

And as tilly already pointed out, why are you incrementing the reference counts of everything every time CLONE is called?

The main problem here is that your are posting do-nothing code that serves only to demonstrate that you are doing something wrong--not what it is that you are actually trying to achieve. That makes it impossible to correct because we cannot infer from what you are doing (wrong), what you should be doing, to make it right. Whatever it is?

Have you read this: http://perldoc.perl.org/perlmod.html#Making-your-module-threadsafe?

Because having played with the code you posted and simplified and simplified, it seems clear from this statement:

CLONE is called correctly, but DESTROY is called to less.

That you are expecting CLONE to be called for each thing (object) cloned, each time it is cloned. And that simply isn't the case. Note (in the docref above) where it says: "CLONE will be called once as a class method for every package that has it defined (or inherits it). It will be called in the context of the new thread".

Ie. For any given package, CLONE will be called on each new thread, once, for each class that is loaded at the time the thread is spawned. Not for each instance!

Conversely, DESTROY(), will be called once on each thread, for each instance that exists.

To demonstrate, this simplified version of your code creates two instances of the class and passes them to a thread, which passes them on to a second thread. Both threads then die before the process is repeated:

Read more... (2 kB)

It produces this output:

C:\test>junk10

MAIN[0]: Creating thread 1 passing objs: Class=HASH(0x2683c0) & Class=
+HASH(0x2c4e50)
>CLONE[1]:
<CLONE[1]: this; Class
THREAD[1] L:0 running
>CLONE[2]:
<CLONE[2]: this; Class
THREAD[2] L:1 running
THREAD[1] L:0 finished
>DESTROY[1]:
<DESTROY[1]: this: Class=HASH(0x3c6dc78)
>DESTROY[1]:
<DESTROY[1]: this: Class=HASH(0x3c6dcc0)
THREAD[2] L:1 finished
>DESTROY[2]:
<DESTROY[2]: this: Class=HASH(0x3d34148)
>DESTROY[2]:
<DESTROY[2]: this: Class=HASH(0x3d34178)
>DESTROY[0]:
<DESTROY[0]: this: Class=HASH(0x2c4e50)
>DESTROY[0]:
<DESTROY[0]: this: Class=HASH(0x2683c0)

MAIN[0]: Creating thread 2 passing objs: Class=HASH(0x2683c0) & Class=
+HASH(0x268120)
...
[download]

Note that although two instance of Class are created by main and passed in turn to each of the two threads, Class::CLONE() is only >entered and <exited once in the context of each of those two threads.

However, Class::DESTROY() is called is called six times! Twice (once for each instance) in the context of each of the spawned threads to destroy the clones created for those threads; and when those have both been destroyed, twice in the context of the main thread (0) to destroy the originals.

---

I'm hoping that will start to make what you are doing wrong in trying to achieve whatever you are trying to achieve clear to you? If that is the case, I'd appreciate feedback both on whether it helped? And what you are trying to achieve? Because whilst the doomsayers have been fumbling their gums with doom and gloom on a subject they obviously know nothing about, I've spent a not inconsiderable portion of the last two days trying to work out what you are really trying to do.

I appreciate that you are trying to track resources, but if the resources are perl allocated--ie. any standard perl allocated resource like file handles; even if allocated from within XS code--then you need to do quite literally nothing, and Perl will take care of their management, including destruction.

But if you are genuinely allocating non-Perl resources on the basis of calls to CLONE(), what are those unmanaged resources? Are you allocating them on a per-thread basis? Or a per-instance basis? Or a per-instance-per-thread basis? Or ... ?

I kept hoping that you would either volunteer some more (useful) information, or that someone would ask you for it before jumping to knee-jerk conclusions; or trotting out the "threads are bad; or hard; or not unix-y; or I-got-bitten-by-them-on-my-first-attempt-when-I-didn't-know-what-I-was-doing-so-now-I-tell-everyone-that-mentions-them-they-are-scarey-and they-shouldn't-use-them" dogmas.

---

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."

Make each shared object keep track of how much it is used. In CLONE, increment the usage of each object that the thread will actually use. In DESTROY, decrement the usage of each object that the thread incremented. The key is that you have to decrement all of the things that you increment.

This should work much, much better than having each thread increment everything, then only decrement itself. Which is what your code currently does.

That said, I would personally advocate avoiding using threads unless you absolutely need to. Threads are always a huge complication, and I'm a big advocate of trying the simplest thing that could possibly work.

Threads are always a huge complication, ...

... "Perl is always slow"; "football fans are always hooligans"; "blonds are always dumb"; and "mindless generalisations are always pointless".

One of the above is true.

---

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."

How is having a computer doing 2 things at once not more complicated than having it do 1? Furthermore figuring out the failure modes generally involves figuring out sequences of different threads hitting specific interactions.

Threads are useful in figuring out how to use the maximum CPU power you can, and for solving latency issues. Threads are sometimes the simplest way of coming up with a solution that does what you want and addresses those issues. In those cases it is appropriate to use threads.

But even in those situations the presence of threads is a complication in the code, and many (probably most) programmers are poorly equipped to properly deal with them. So I'm never going to suggest a multi-threaded solution unless there is a specific reason to think that they address a real problem which you have.

That said, I would personally advocate avoiding using threads unless you absolutely need to.

Most of my programs include server and other working threads.

Make each shared object keep track of how much it is used. In CLONE, increment the usage of each object that the thread will actually use. In DESTROY, decrement the usage of each object that the thread incremented. The key is that you have to decrement all of the things that you increment.

I tried it out, but it doesn't seem to be working better.

Threads are always a huge complication

Looks like you are right. The code below could work better. But... either Perl crashes or threads don't finish anymore. Very strange.

#!/usr/local/bin/perl

use threads;

use Time::HiRes qw/usleep/;

while( 1 ) {
    my $class = Class->new();
    threads->create( \&thread_sub, $class, 0 );
    usleep 200_000;
}

sub thread_sub {
    my( $class, $level ) = @_;
    print "THREAD ", threads->self->tid, ", $level running\n";
    threads->create( \&thread_sub, $class, $level + 1 ) unless $level;
    usleep 400_000;
    print "THREAD ", threads->self->tid, ", $level finished\n";
    threads->self->detach;
}

1;

package Class;

use threads::shared;

our %MY_CXT : shared = ();
our $CXT_ID : shared = 0;

sub new {
    my $class = shift;
    my $this = {};
    $this->{'shared'} = &share( {} ),
    $this->{'shared'}{'refcnt'} = 1;
    $this->{'shared'}{'tid'} = threads->self->tid;
    $this->{'shared'}{'clone'} = 1;
    lock( %MY_CXT );
    $this->{'id'} = ++ $CXT_ID;
    $MY_CXT{$this->{'id'}} = $this->{'shared'};
    bless $this, $class;
}

sub CLONE {
    lock( %MY_CXT );
    while( my( $k, $v ) = each %MY_CXT ) {
        # do not CLONE anymore when it's destroyed in the main thread
        $v->{'refcnt'} ++ if $v->{'clone'};
        print "CLONE: id $k, refcnt $v->{'refcnt'}\n";
    }
}

sub DESTROY {
    my $this = shift;
    my $shared = $this->{'shared'};
    if( $shared->{'tid'} == threads->self->tid ) {
        # disable futher CLONEs
        $shared->{'clone'} = 0;
        # refcnt must decrement twice ?
        $shared->{'refcnt'} --;
    }
    $shared->{'refcnt'} --;
    print "DESTROY: id $this->{'id'}, refcnt $shared->{'refcnt'}\n";
    if( $shared->{'refcnt'} <= 0 ) {
        lock( %MY_CXT );
        # should not happen
        warn "already destroyed!!!" unless $MY_CXT{$this->{'id'}};
        delete $MY_CXT{$this->{'id'}};
    }
}

1;
[download]