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.

Neither it seems is ikegami's original suggestion:

We knew that two days ago, so we fixed it two days ago.

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.

Thanks for fixing the code but I guess you are missing the point. My contention in an earlier message was that this piece of code by BrowserUk is not thread safe:

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

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

My reasoning of it was as follows:

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]

In other words, the problem with the above safe_set implementation is that it creates a new $v so threads could be waiting on different "copies" of $v, thereby causing a race condition.

To test out safe_set implementation, I made the hash %h to have a single key and tried updates using several threads. Notice that I copy and then add one to it. If there is a race condition the value of variable is surely affected. The code did not run at all and failed giving out a MUTEX_LOCK error: So, the real question here is if my testing code (given below) really points to a problem with the safe_set function. The way you have implemented it basically changes the flavor of safe_set, which of course runs.. I hope you are able to see my point here.

#! 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]

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]