That is a pretty nice solution, except of course it consumes ...

... an entire mirror data structure, completely unnecessarily.

Why not store the values in your existing hash using references to shared scalars, and lock the individual scalars directly rather than via a proxy?

sub safe_set {
   my $k = shift;
   my $v :shared = shift;

   lock $$h{ $k };
   $h{$k} = \$v;
}
[download]

The reasons why you can't lock individual hash elements are quite involved, but they boil down to the facts that:

• the values in a hash are not indexed directly by the keys themselves. And the keys aren't stored internally in scalars.
• the values in shared hashes aren't themselves shared scalars by default.

A full explanation would probably require the original author to explain, but it probably comes down to the path of least resistance.

Thanks! Fascinating solution except that I think it is not thread-safe..

TIMESTEP 0:

    Say, $h{$k} points to v1 
    Thread T0: Holding lock on v1
    Thread T1: Waiting on Lock on v1 

TIMESTEP 1:

    T0 changes $h{$k} = \$v2, releases lock on $v1

TIMESTAMP 2: (Thread T2 makes an entrance)

    Thread T2: Acquires Lock on v2, proceeds
    Thread T1: Acquires Lock on v1, proceeds
    <Race-around condition>
[download]

I've tried everything I can think of to provoke this 'race condition', but I've yet to see it manifest itself in real code:

#! perl -slw
use strict;
use Data::Dump qw[ pp ];
use Time::HiRes qw[ time ];
use List::Util qw[ shuffle ];
use threads;
use threads::shared;

our $START //= 'aaa';
our $END   //= 'zzz';

my %hash :shared = map{ my $n :shared = 0; $_ => \$n } $START .. $END;

my $start = time;
$_->join for map{
    async{
        my @order;
        {
            lock %hash;
            @order = shuffle keys %hash;
        }
        for ( @order ) {
            lock ${ $hash{ $_ } };
            ++${ $hash{ $_ } };
        }
    }
} 1 .. 40;

printf "Lock scalar ref took %.3f seconds\n", time() - $start;

my @fails = grep $$_ != 40, values %hash;
warn @fails . " fails\n" if @fails;

__END__
C:\test>909437-2
Lock scalar ref took 42.807 seconds
[download]

By all means show me how?

---

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.

Neither it seems is ikegami's original suggestion:

#! perl -slw
use strict;
use Data::Dump qw[ pp ];
use Time::HiRes qw[ time ];
use List::Util qw[ shuffle ];
use threads;
use threads::shared;

our $START //= 'aaa';
our $END   //= 'zzz';

my %sems :shared = map{ my $n :shared = 0; $_ => \$n } $START .. $END;
my %hash :shared = map{ $_ => 0 } $START .. $END;

my $start = time;
$_->join for map{
    async{
        my @order = shuffle keys %hash;
        for ( @order ) {
            lock ${ $sems{ $_ } };
            ++$hash{ $_ };
        }
    }
} 1 .. 4;

printf "Lock scalar ref took %.3f seconds\n", time() - $start;

my @fails = grep $_ != 4, values %hash;

warn @fails . " fails\n" if @fails;

__END__
C:\test>909437-3
Lock scalar ref took 2.937 seconds
2840 fails
[download]

---

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.

This is my safe set exerciser, which does crashes giving out the following error.

Thread <X> terminated abnormally: panic: MUTEX_LOCK (22) shared.xs:199 at ./thread_test.pl line 25.

I checked this multiple times but did not find anything wrong with it. May be a problem with the safe_set as I indicated in my post or may be not?

#! perl -slw

use strict;
use threads;
use threads::shared;

our %h : shared;

# Setting up just a single row to increase chances 
# a race condition
our $k = 'AAA';
our $val : shared = 0;
our $THREADS = 50;
our $iter = 50;
$h{$k} = \$val;

# Safe_set similar to BrowserUk 
sub safe_set {
  my $v :shared;
  # Critical Section
  {
     lock ${$h{$k}};
     $v = ${$h{$k}} + 1;
     $h{$k} = \$v;
  }
}

# Keep locking to increment $$h{$k}
sub test_safe_set {
  for (my $i = 0; $i < $iter; ++$i) {
     safe_set();
  }
}

my @pool = map{
      threads->create(\&test_safe_set)
    } 1 .. $THREADS;

$_->join for @pool;

warn ${$h{$k}}, "failed\n" if ${$h{$k}} != $THREADS * $iter;
[download]

Try it this way:

#! perl -slw

use strict;
use threads;
use threads::shared;

our %h : shared;

# Setting up just a single row to increase chances
# a race condition
our $k = 'AAA';
our $val : shared = 0;
our $THREADS = 50;
our $iter = 50;
$h{$k} = \$val;

# Safe_set similar to BrowserUk
sub safe_set {
  # Critical Section
  {
     lock ${$h{$k}};
      ${ $h{$k} } = ${ $h{$k}   } + 1;
  }
}

# Keep locking to increment $$h{$k}
sub test_safe_set {
  for (my $i = 0; $i < $iter; ++$i) {
     safe_set();
  }
}

my @pool = map{
      threads->create(\&test_safe_set)
    } 1 .. $THREADS;

$_->join for @pool;

warn ${$h{$k}}, "failed\n" if ${$h{$k}} != $THREADS * $iter;
[download]

---

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.

Dunno, but safe_set does a lot of work for nothing. It assumes $h{$k} is already a reference to a shared var, so why does it create a new one?

sub safe_inc {
  lock ${$h{$k}};
  ${$h{$k}} = ${$h{$k}} + 1;
}
[download]

except of course it consumes a mutex variable per row.

As requested.

BTW, I think the following line in your code is not thread safe;

oh, true! The fix is good, but the "defined" are superfluous.