salva has asked for the wisdom of the Perl Monks concerning the following question:

Let me start by saying that this post is about fixing the bug in Net::SSH2 explained here and here.

The issue I am facing is that Net::SSH2 wraps the objects in the underlaying libssh2 library using the sv_setref_pv function that just creates a new SV and places the pointer into the IV slot. When a thread containing Net::SSH2 objects creates a new thread, those SVs are cloned but the objects they point to are not. Later, when in some thread the object gets out of scope and its DESTROY method is called, the underlaying object is deallocated leaving the clones with a dangling pointer and after that, any use of those, including destroying then, may cause the application to crash.

So, my question is, what mechanism could be used to avoid that. How XS/C objects can be wrapped in a multi-thread friendly way?

The only viable way I can envision in to use magic for that, replacing the sv_setref_pv wrapping for one using a magic structure of type PERL_MAGIC_ext (~) with svt_dup pointing to a callback that does reference counting.

Are there any easier ways for doing it?

Replies are listed 'Best First'.
Re: Multi-thread friendly object life-time handling for XS modules
by syphilis (Archbishop) on Nov 06, 2017 at 12:54 UTC
    So, my question is, what mechanism could be used to avoid that

    This is quite possibly a non-helpful response, but I think that "blessing" the new SV into package NULL is one way of avoiding the issue.
    This effectively means that when the SV goes out of scope, it gets garbage-collected, but the object it points to survives (because DESTROY wasn't called). Therefore, any other SV's that point to the same object are still valid.
    This also effectively means that the programmer has to take charge of determining if/when these objects are DESTROY()ed - or run the risk of memory leaks.

    For some reason that I've never understood, I provided in Math::MPFR the capability of creating such unblessed objects and it does seem to me to provide a way of avoiding the problem. With an object blessed into package Math::MPFR: 
    use warnings;
use strict;
use threads;
use Math::MPFR qw(:mpfr);

my ($x, $inex) = Rmpfr_init_set_d(11.625, MPFR_RNDN);

my $thr1 = threads->create(
             sub {
                 print Rmpfr_get_d($x, MPFR_RNDN), "\n";
                 return 23;
                 } );       ## Line 12

my $res = $thr1->join();
print $res;

__END__

OUTPUTS:
11.625
Free to wrong pool 4ab7e0 not 49aee0 at threads.pl line 12.

    [download]
    With the unblessed object: 
    use warnings;
use strict;
use threads;
use Math::MPFR qw(:mpfr);

my ($x, $inex) = Rmpfr_init_set_d_nobless(11.625, MPFR_RNDN);

my $thr1 = threads->create(
             sub {
                 print Rmpfr_get_d($x, MPFR_RNDN), "\n";
                 return 23;
                 } );

my $res = $thr1->join();
Math::MPFR::DESTROY($x);
print $res;

__END__

OUTPUTS:
11.625
23

    [download]
    Maybe that's an overly simplistic and/or irrelevant demo. (My knowledge of threads is very week.)

    Cheers,
    Rob
[reply]
[d/l]
[select]
Re: Multi-thread friendly object life-time handling for XS modules
by BrowserUk (Patriarch) on Nov 06, 2017 at 13:45 UTC

    I once considered trying to use the UNTIE() method (see the end of the untie gotcha in the perltie manpage), for this purpose, but never pursued it. Might be worth a look.

    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". The enemy of (IT) success is complexity.
    In the absence of evidence, opinion is indistinguishable from prejudice. Suck that fhit
[reply]
Re: Multi-thread friendly object life-time handling for XS modules
by Anonymous Monk on Nov 06, 2017 at 10:30 UTC
[reply]
      Wx is a big and complex module, but after a brief inspection, it seems to me it is just using the non thread-safe sv_setref_pv function to create its wrapper objects.

      DBI uses the svt_dup approach.

[reply]
[d/l]
[select]
Re: Multi-thread friendly object life-time handling for XS modules
by Anonymous Monk on Nov 06, 2017 at 19:39 UTC

    You could probably fix it on the perl side by dropping a shared ref into the object and testing that in DESTROY. 

    my $OB = bless { piggy => shared_clone([]) };
my $T = async { };

say threads::shared::_refcnt($OB->{piggy});
$T->join;
say threads::shared::_refcnt($OB->{piggy});

    [download]

[reply]
[d/l]
Re: Multi-thread friendly object life-time handling for XS modules
by Anonymous Monk on Nov 09, 2017 at 01:35 UTC
    This is probably a very-fundamental design issue that needs to be taken up with the module's developers: "the thing is apparently not thread-safe at all, given its present internal design."
[reply]
      Nowadays, the module's developers are mostly, me :-)
[reply]

Back to Seekers of Perl Wisdom