Complicated shared variable bugs & Test::Builder.

[Michael G Schwern]

In repairing Test::Builder to work with ithreads again I ran into some
broken parts of threads::shared. These things only manifest themselves as
part of Test::Builder, I can't get them to break otherwise when used in
small programs.

I had to do some pretty funny work-arounds to get things to
work. Let's look at some of the rough spots.

skip(), todo_skip() and current_test() all contain awkward code like this:

my %result;
share(%result);
%result = (
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
);
$Test_Results[$Curr_Test-1] = \%result;

(I can't use the shared attribute because of backwards compatibility).

this was originally:

$Test_Results[$Curr_Test-1] = {
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
};

it would be Very Nice if Perl could recognize that since @Test_Results
is shared and we're assigning an anonymous hash ref it should DWIM and make
that reference shared without my having to explicitly say so.

Since it currently doesn't, I tried this:

$Test_Results[$Curr_Test-1] = &share({
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
});

but, as mentioned in another bug, share() always returns a list and we wind
up with 1 so that's not going to work. How about this then:

$Test_Results[$Curr_Test-1] = (&share({
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
}))[0];

that should work. It doesn't. For some reason it winds up with nothing
assigned.

Test::Builder::skip(lib/Test/Builder.pm:665):
665: $Test_Results[$Curr_Test-1] = (&share({
666: 'ok' => 1,
667: actual_ok => 1,
668: name => '',
669: type => 'skip',
670: reason => $why,
DB<2>
Test::Builder::skip(lib/Test/Builder.pm:674):
674: my $out = "ok";
DB<2> x \@Test_Results
0 ARRAY(0x103a251c)
0 HASH(0x10396cbc)
'actual_ok' => 1
'name' => 'no tests yet, no summary'
'ok' => 1
'reason' => ''
'type' => ''
1 HASH(0x10396d10)
empty hash
DB<3> x $Curr_Test
0 2

Okay, how about this:

my($result) = &share({
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
});
$Test_Results[$Curr_Test-1] = $result;

nope. Empty again.

Test::Builder::skip(lib/Test/Builder.pm:665):
665: my($result) = &share({
666: 'ok' => 1,
667: actual_ok => 1,
668: name => '',
669: type => 'skip',
670: reason => $why,
DB<2>
Test::Builder::skip(lib/Test/Builder.pm:672):
672: $Test_Results[$Curr_Test-1] = $result;
DB<2> x $result
0 HASH(0x104a2fc8)
empty hash

maybe it's something to do with assigning straight to my().

my $result;
($result) = &share({
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
});
$Test_Results[$Curr_Test-1] = $result;

Nope, empty again.

Ok, let's try sharing the scalar then assigning to it:

my $result;
share($result);
$result = {
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
};
$Test_Results[$Curr_Test-1] = $result;

that simply bombs at the assignment to $result.

Invalid value for shared scalar at lib/Test/Builder.pm line 667.

so I'm left with the very explicit and drawn out:

my %result;
share(%result);
%result = (
'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
);
$Test_Results[$Curr_Test-1] = \%result;

Down in _ending() we see this:

# 5.8.0 threads bug. Shared arrays will not be auto-extended
# by a slice. Worse, we have to fill in every entry else
# we'll get an "Invalid value for shared scalar" error
for my $idx ($#Test_Results..$Expected_Tests-1) {
my %empty_result = ();
share(%empty_result);
$Test_Results[$idx] = \%empty_result
unless defined $Test_Results[$idx];
}

my $num_failed = grep !$_->{'ok'}, @Test_Results[0..$Expected_Tests-1];
$num_failed += abs($Expected_Tests - @Test_Results);

this was originally:

# 5.8.0 threads bug. Shared arrays will not be auto-extended
# by a slice.
$Test_Results[$Expected_Tests-1] = undef
unless defined $Test_Results[$Expected_Tests-1];

my $num_failed = grep !$_->{'ok'}, @Test_Results[0..$Expected_Tests-1];
$num_failed += abs($Expected_Tests - @Test_Results);

but it would choke on the grep with an "Invalid value for shared scalar".

$ perl5.8.0-ithreads -Ilib -w t/missing.t
1..2
Invalid value for shared scalar at lib/Test/Builder.pm line 1329.
END failed--call queue aborted.
ok 1
not ok 2

We're already working around one bug, a shared array will not be extended by
a slice. You have to do it explicitly.

This bug is somewhat tricky. It only appears when trying to dereference $_.
Simply using $_ doesn't cause a problem.

It's also not sufficient to simply fill in the extended slots with shared
scalars containing undefs:

# 5.8.0 threads bug. Shared arrays will not be auto-extended
# by a slice. Worse, we have to fill in every entry else
# we'll get an "Invalid value for shared scalar" error
for my $idx ($#Test_Results..$Expected_Tests-1) {
my $empty_result;
share($empty_result);
$empty_result = undef;
$Test_Results[$idx] = $empty_result
unless defined $Test_Results[$idx];
}

my $num_failed = grep !$_->{'ok'}, @Test_Results[0..$Expected_Tests-1];
$num_failed += abs($Expected_Tests - @Test_Results);

but that chokes all the same.

It's also not enough to only fill the last slot with a hash ref:

my %empty_result = ();
share(%empty_result);
$Test_Results[$Expected_Tests-1] = \%empty_result
unless defined $Test_Results[$Expected_Tests-1];

my $num_failed = grep !$_->{'ok'}, @Test_Results[0..$Expected_Tests-1];
$num_failed += abs($Expected_Tests - @Test_Results);

Test::Builder::_ending(lib/Test/Builder.pm:1318):
1318: my %empty_result = ();
DB<3> n
Test::Builder::_ending(lib/Test/Builder.pm:1319):
1319: share(%empty_result);
DB<3>
Test::Builder::_ending(lib/Test/Builder.pm:1320):
1320: $Test_Results[$Expected_Tests-1] = \%empty_result
1321: unless defined $Test_Results[$Expected_Tests-1];
DB<3>
Test::Builder::_ending(lib/Test/Builder.pm:1323):
1323: my $num_failed = grep !$_->{'ok'}, @Test_Results[0..$Expected_Tests-1];
DB<3> x \@Test_Results
0 ARRAY(0x10431db8)
0 HASH(0x103f98c4)
'actual_ok' => 1
'name' => 'Foo'
'ok' => 1
'reason' => ''
'type' => ''
1 HASH(0x103f9918)
'actual_ok' => 0
'name' => 'Bar'
'ok' => 0
'reason' => ''
'type' => ''
2 empty slot
3 empty slot
4 HASH(0x103f996c)
empty hash
DB<4> c
Invalid value for shared scalar at lib/Test/Builder.pm line 1323.
END failed--call queue aborted.
ok 1
not ok 2
Debugged program terminated. Use q to quit or R to restart,

They have to all be filled with hash refs. Only then will it work.

So that's the problems I found while getting Test::Simple/More/Builder
playing nice with ithreads. I can't get these bugs to manifest themselves
when I try the above techniques in short programs. Those interested in
trying to reproduce the problems can download 0.47 and try to improve the
sharing code in skip() and _ending(). When you do, things will break.
t/details.t and t/missing.t are the easiest tests to use to exercise this.

--

Michael G. Schwern <sch...@pobox.com> http://www.pobox.com/~schwern/
Perl Quality Assurance <per...@perl.org> Kwalitee Is Job One
There is nothing wrong. We have taken control of this sig file. We will
return it to you as soon as you are groovy.

[Elizabeth Mattijsen]

At 04:46 AM 8/26/02 -0700, Michael G Schwern wrote:
>In repairing Test::Builder to work with ithreads again I ran into some
>broken parts of threads::shared. These things only manifest themselves as
>part of Test::Builder, I can't get them to break otherwise when used in
>small programs.

Welcome to the world of Perl threads.

Maybe it's an idea to use Thread::Tie for Test::Builder? It has shared
arrays with all the features of normal arrays at the expense of CPU. The
current behaviour of shared arrays was one of the reasons for writing
Thread::Tie in the first place.

Thread::Tie currently also implies Thread::Serialize and Storable. The
latter is part of 5.8.0, so there no real extra dependency there. And I'm
hoping that Thread::Serialize will become a no-op in future versions of
Perl: I think you _should_ be able to copy data structures between threads
without having to think about it.

Liz

[Benjamin Goldberg]

Michael G Schwern wrote:
[snip]

> Ok, let's try sharing the scalar then assigning to it:
>
> my $result;
> share($result);

This makes a shared scalar.

> $result = {
> 'ok' => 1,
> actual_ok => 1,
> name => '',
> type => 'skip',
> reason => $why,
> };

This tries to store an unshared hash into the shared scalar.

> $Test_Results[$Curr_Test-1] = $result;
>
> that simply bombs at the assignment to $result.
>
> Invalid value for shared scalar at lib/Test/Builder.pm line 667.

Try doing:

my $result = {

'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
};

share($result); # or maybe &share($result);
$Test_Results[$Curr_Test-1] = $result;

This is probably a rather silly idea, but have you considered prefilling
(before running any tests, and of course before creating any threads)
@Test_Results with a bunch of empty, shared hashrefs, and later doing
the assignments as:

%{ $Test_Results[$Curr_Test-1] } = (

'ok' => 1,
actual_ok => 1,
name => '',
type => 'skip',
reason => $why,
);

Since the contents of @Test_Results would *already* be shared, this part
of the code should work identically for threaded and unthreaded perls.

And on unthreaded perls, you could skip the prefilling, since
autovivification would take place.

--
tr/`4/ /d, print "@{[map --$| ? ucfirst lc : lc, split]},\n" for
pack 'u', pack 'H*', 'ab5cf4021bafd28972030972b00a218eb9720000';

[Michael G Schwern]

On Mon, Aug 26, 2002 at 03:04:35PM +0200, Elizabeth Mattijsen wrote:
> At 04:46 AM 8/26/02 -0700, Michael G Schwern wrote:
> >In repairing Test::Builder to work with ithreads again I ran into some
> >broken parts of threads::shared. These things only manifest themselves as
> >part of Test::Builder, I can't get them to break otherwise when used in
> >small programs.
>
> Welcome to the world of Perl threads.
>
> Maybe it's an idea to use Thread::Tie for Test::Builder?

Thank you, but I've already got work arounds. I'd rather see the internal
thread bugs fixed.

--

Michael G. Schwern <sch...@pobox.com> http://www.pobox.com/~schwern/
Perl Quality Assurance <per...@perl.org> Kwalitee Is Job One

I'm going to have to hurt you on principle.

[Michael G Schwern]

On Mon, Aug 26, 2002 at 04:56:03PM -0400, Benjamin Goldberg wrote:
> Michael G Schwern wrote:
> [snip]
> > Ok, let's try sharing the scalar then assigning to it:
> >
> > my $result;
> > share($result);
>
> This makes a shared scalar.
>
> > $result = {
> > 'ok' => 1,
> > actual_ok => 1,
> > name => '',
> > type => 'skip',
> > reason => $why,
> > };
>
> This tries to store an unshared hash into the shared scalar.

This works fine in a simple case.

#!/usr/bin/perl -lw

use threads::shared;

my $result;
share($result);
$result = {

foo => 42
};
print $result->{foo};

my @foo;
share(@foo);
$foo[0] = $result;
print $foo[0]{foo};

It only causes a problem when used in complicated code like Test::Builder.

> This is probably a rather silly idea, but have you considered prefilling
> (before running any tests, and of course before creating any threads)
> @Test_Results with a bunch of empty, shared hashrefs

Can't do that, don't know now long @Test_Results is going to be beforehand.

Besides, I shouldn't have to do silly work arounds like that. Thus the
whole point of the post. These are Bugs and/or Annoyances which only
manifest themselves in complex code.

--

Michael G. Schwern <sch...@pobox.com> http://www.pobox.com/~schwern/
Perl Quality Assurance <per...@perl.org> Kwalitee Is Job One

local $variable for local @people;

[Arthur Bergman]

On tisdag, augusti 27, 2002, at 01:11 , Arthur Bergman wrote:

>
> $foo = share({});
>
> will do the right thing in both shared and non shared enviroments
>
>

That would be

$foo = &share({));

Arthur

[Nick Ing-Simmons]

Michael G Schwern <sch...@pobox.com> writes:
>
>it would be Very Nice if Perl could recognize that since @Test_Results
>is shared and we're assigning an anonymous hash ref it should DWIM and make
>that reference shared without my having to explicitly say so.

Agreed. The main reason I didn't do that is for things like

$Test_Results[$Curr_Test-1] = { Foo => \@Test_results } ;

I was not the mood to code "safe" recursive check.

It might be easier to handle the {} case using attributes - so that
we knew at compile time that this {} was destined for a shared thingy.

>
>Since it currently doesn't, I tried this:
>
> $Test_Results[$Curr_Test-1] = &share({
> 'ok' => 1,
> actual_ok => 1,
> name => '',
> type => 'skip',
> reason => $why,
> });
>
>but, as mentioned in another bug, share() always returns a list and we wind
>up with 1 so that's not going to work. How about this then:

These are probably sharing the ref not the hash (or vice-versa).

Shared has a \[$%@] "prototype" and {} is a ref to hash.

>
> $Test_Results[$Curr_Test-1] = (&share({
> 'ok' => 1,
> actual_ok => 1,
> name => '',
> type => 'skip',
> reason => $why,
> }))[0];
>
>that should work. It doesn't. For some reason it winds up with nothing
>assigned.
>

--
Nick Ing-Simmons
http://www.ni-s.u-net.com/

[H.Merijn Brand]

$foo = &share ({});

> Arthur

--
H.Merijn Brand Amsterdam Perl Mongers (http://amsterdam.pm.org/)
using perl-5.6.1, 5.8.0 & 633 on HP-UX 10.20 & 11.00, AIX 4.2, AIX 4.3,
WinNT 4, Win2K pro & WinCE 2.11. Smoking perl CORE: smo...@perl.org
http://archives.develooper.com/daily...@perl.org/ per...@perl.org
send smoke reports to: smokers...@perl.org, QA: http://qa.perl.org

[Arthur Bergman]

On tisdag, augusti 27, 2002, at 01:09 , sch...@pobox.com wrote:

>
> Doing it with attributes means I can't use it if I want to remain
> backwards compatible. :( It also means it's not DWIM. Dealing with
> complex, shared data structures really shouldn't be that complicated
> for the user.
>
>

$foo = share({});

will do the right thing in both shared and non shared enviroments

arthur

[sch...@pobox.com]

On Tue, Aug 27, 2002 at 12:03:15PM +0100, Nick Ing-Simmons wrote:
> Agreed. The main reason I didn't do that is for things like
>
> $Test_Results[$Curr_Test-1] = { Foo => \@Test_results } ;
>
> I was not the mood to code "safe" recursive check.
>
> It might be easier to handle the {} case using attributes - so that
> we knew at compile time that this {} was destined for a shared thingy.

Doing it with attributes means I can't use it if I want to remain

backwards compatible. :( It also means it's not DWIM. Dealing with
complex, shared data structures really shouldn't be that complicated
for the user.

> >Since it currently doesn't, I tried this:
> >
> > $Test_Results[$Curr_Test-1] = &share({
> > 'ok' => 1,
> > actual_ok => 1,
> > name => '',
> > type => 'skip',
> > reason => $why,
> > });
> >
> >but, as mentioned in another bug, share() always returns a list and we wind
> >up with 1 so that's not going to work. How about this then:
>
> These are probably sharing the ref not the hash (or vice-versa).
>
> Shared has a \[$%@] "prototype" and {} is a ref to hash.

The docs explicitly say the prototype is really a bug and that you can
work around it with &share if you want to do a reference and that
share() will look into references.

So I believe I'm using it correctly, according to the docs.

--
Michael G Schwern <sch...@pobox.com> http://www.pobox.com/~schwern/

[Elizabeth Mattijsen]

At 01:44 PM 8/27/02 +0200, H.Merijn Brand wrote:
> $foo = &share ({});

Hmmm... this is getting scary:

$ perl -e -Mthreads -Mthreads::shared 'print $foo = &share ({})'
(doesn't print anything)

$ perl -e -Mthreads -Mthreads::shared '$foo = &share ({}); print $foo'
(doesn't print anything)

$ perl -e -Mthreads -Mthreads::shared '$foo = {}; print $foo'
(doesn't print anything)

$ perl -e '$foo = {}; print $foo'
HASH(0x8125ed4)

Maybe the problem isn't in the share()ing at all? But in just having
threads::shared loaded?

Liz

[Rafael Garcia-Suarez]

Elizabeth Mattijsen wrote:
> At 01:44 PM 8/27/02 +0200, H.Merijn Brand wrote:
>
>> $foo = &share ({});
>
>
> Hmmm... this is getting scary:
>
> $ perl -e -Mthreads -Mthreads::shared 'print $foo = &share ({})'

^^^^^^^^^
This is your script. The remaning command-line words are
put in @ARGV.
With -w you get "Useless use of negation (-) in void context".

[Elizabeth Mattijsen]

At 02:21 PM 8/27/02 +0200, Rafael Garcia-Suarez wrote:
>>$ perl -e -Mthreads -Mthreads::shared 'print $foo = &share ({})'

>This is your script. The remaning command-line words are
>put in @ARGV.

Indeed. Just goes to show I don't do -e much...

>With -w you get "Useless use of negation (-) in void context".

Which now completely makes sense to me. Thanks for the explanation to the
rooky...

Liz

[Elizabeth Mattijsen]

Of course, I should have run this with warnings:

$ perl -we '$foo = {}; print $foo'
HASH(0x8125ed4)

$ perl -we -Mthreads -Mthreads::shared '$foo = {}; print $foo'
Useless use of negation (-) in void context at -e line 1.

????

Getting stranger and stranger. Or am I missing something?

Liz

[Abigail]

Yes.

$ perl -MO=Deparse -we -Mthreads

Useless use of negation (-) in void context at -e line 1.

BEGIN { $^W = 1; }
-'Mthreads';
-e syntax OK
$ perl -we -Mthreads -e ';print "@ARGV\n"' -Mthreads::shared '$foo = {}; pri

nt $foo'
Useless use of negation (-) in void context at -e line 1.

$foo = {}; print $foo
$


The body of your program is "-Mthreads", with @ARGV containing
'-Mthreads::shared' and '$foo = {}; print $foo'.

"-Mthreads" parses as "- 'Mthreads'", and use of unary negation in
void context gives the warning you saw.



Abigail

[Slaven Rezic]

Elizabeth Mattijsen <l...@dijkmat.nl> writes:

Just put -e before the code.

Regards,
Slaven

--
Slaven Rezic - slaven...@berlin.de
BBBike - route planner for cyclists in Berlin
WWW version: http://www.bbbike.de
Perl/Tk version: http://bbbike.sourceforge.net

[Nick Ing-Simmons]

Elizabeth Mattijsen <l...@dijkmat.nl> writes:
>Of course, I should have run this with warnings:
>
>
>$ perl -we '$foo = {}; print $foo'
>HASH(0x8125ed4)
>
>$ perl -we -Mthreads -Mthreads::shared '$foo = {}; print $foo'

You meant

perl -w -Mthreads -Mthreads::shared -e '$foo = {}; print $foo'

that is -M's have to be there _before_ the -e

>Useless use of negation (-) in void context at -e line 1.
>
>????
>
>
>Getting stranger and stranger. Or am I missing something?
>
>
>Liz

[Michael G Schwern]

On Tue, Aug 27, 2002 at 01:11:34PM +0200, Arthur Bergman wrote:
> $foo = share({});
>
> will do the right thing in both shared and non shared enviroments

It *should* but it doesn't. See the original post about the problems I had
in Test::Builder.

--

Michael G. Schwern <sch...@pobox.com> http://www.pobox.com/~schwern/

Perl Quality Assurance <per...@perl.org> Kwalitee Is Job One

Don't step on my funk

[Michael G Schwern]

On Tue, Aug 27, 2002 at 03:47:54PM -0700, Michael G Schwern wrote:
> On Tue, Aug 27, 2002 at 01:11:34PM +0200, Arthur Bergman wrote:
> > $foo = share({});
> >
> > will do the right thing in both shared and non shared enviroments
>
> It *should* but it doesn't. See the original post about the problems I had
> in Test::Builder.

Forget Test::Builder, here's a one-liner.

$ perl5.8.0-ithreads -Mthreads::shared -wle '$foo = share({}); print $foo'
1

That's the share() returns a list problem.

$ perl5.8.0-ithreads -Mthreads::shared -wle '($foo) = share({}); print $foo'
HASH(0x10134184)

but even worse when used inside Test::Builder the return value of share() is
an empty list.
http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2002-08/msg01300.html

--

Michael G. Schwern <sch...@pobox.com> http://www.pobox.com/~schwern/
Perl Quality Assurance <per...@perl.org> Kwalitee Is Job One

Woah, like, did anybody see my watch?

[Elizabeth Mattijsen]

At 04:08 PM 8/27/02 -0700, Michael G Schwern wrote:
>Forget Test::Builder, here's a one-liner.
>$ perl5.8.0-ithreads -Mthreads::shared -wle '$foo = share({}); print $foo'
>1

Aha! You're not using threads.pm before using threads::shared! According
to the docs, you should!

$ perl -Mthreads::shared -wle '$foo = share({}); print $foo'
1

$ perl -Mthreads -Mthreads::shared -wle '$foo = share({}); print $foo'
Type of arg 1 to threads::shared::share must be one of [$@%] (not single
ref constructor) at -e line 1, near "})"
Execution of -e aborted due to compilation errors.

$ perl -Mthreads -Mthreads::shared -wle '$foo = &share({}); print $foo'
HASH(0x8125ed4)

Seems like share() is not what we expect to be when threads.pm is not
loaded before threads::shared.pm is loaded.

Liz

[Michael G Schwern]

On Wed, Aug 28, 2002 at 09:46:56AM +0200, Elizabeth Mattijsen wrote:
> At 04:08 PM 8/27/02 -0700, Michael G Schwern wrote:
> >Forget Test::Builder, here's a one-liner.
> >$ perl5.8.0-ithreads -Mthreads::shared -wle '$foo = share({}); print $foo'
> >1
>
> Aha! You're not using threads.pm before using threads::shared! According
> to the docs, you should!

<sniker-snatch>

> Seems like share() is not what we expect to be when threads.pm is not
> loaded before threads::shared.pm is loaded.

The disabled versions have the wrong prototype and the wrong return values.
Also, a disabled version of lock() is missing. I think this will explain a
lot of the troubles I had with Test::Builder, since most of my testing is
done without "use threads".

Patch coming.

--

Michael G. Schwern <sch...@pobox.com> http://www.pobox.com/~schwern/
Perl Quality Assurance <per...@perl.org> Kwalitee Is Job One

It's Airplane Glue sniffing time!

[Dave Mitchell]

On Wed, Aug 28, 2002 at 03:37:39AM -0700, Michael G Schwern wrote:
> The disabled versions have the wrong prototype and the wrong return values.
> Also, a disabled version of lock() is missing. I think this will explain a
> lot of the troubles I had with Test::Builder, since most of my testing is
> done without "use threads".

IIRC, lock() is a built-in, so doesn't need a disabled version.

--
That he said that that that that is is is debatable, is debatable.