Re: Invalid value for shared scalar

2004-10-15 Thread Michael G Schwern
Here's how it is as of 0.49.

If you want Test::More to be thread safe you have to "use threads"
yourself and do it before you use Test::More.  This is a change from
previous versions.  Previously I took the view that users will
probably get this wrong so I'll just do it for them.  The change was
because if T::M always turned threads on when available there were
certain things that became untestable.  This occured to me when I
tried to test the parts of threads::shared that happen when threads
are disabled.  It is very important that a testing module not modify
the environment it is testing.

I fixed the problem with names as references.  I find it kind of
annoying that I had to but I don't really understand threading nuances
to judge who's bug it is.

I think its nasty how many hoops Test::Builder has to jump through to
make itself thread safe especially considering that it never uses
threads itself.  This was 5.8.0 when I did it.  Things have hopefully
improved since then but I still have to support 5.8.0.  Alas.


Re: Invalid value for shared scalar

2004-08-27 Thread Andrew Pimlott
On Fri, Aug 27, 2004 at 12:06:47PM -0700, chromatic wrote:
> It doesn't matter how *you* see Perl threads if users who may run your
> tests see them as worth using

First, as I said, I agree with you "ideally".  I understand all the
points you're making, and they're basically valid.  But the reality is,
there's a trade-off:  By making it more convenient for people using
threads, we increase the probability of bugs for everyone.

Second, you're overblowing the problems with T::B not being thread-safe.
If my tests look like

use T::B;
use_ok('Module::Using::Threads');
ok(M::U::T::function_that_uses_threads);
ok(M::U::T::function_that_talks_to_threads);
...

it doesn't matter if T::B is thread-safe, because all of the calls to
T::B are in a single thread.  I'm not saying you can't write tests that
use T::B from multiple threads, just that most people probably don't.

In other words, it's not like using a threaded perl or even threaded
libraries suddenly requires all code to be thread safe.  Your claim that
my tests may break when run on a threaded perl is mistaken.

Third, it is not outlandish to require people to explicitly ask for
thread-safe behavior.  This is common in the C world.  It's not a good
thing, but it's not a disaster, especially as long as most perl code
doesn't use threads.

Anyway, I accept the decision against me, and hopefully there won't be
any more thread bugs in T::B, so this will never come up again!

Andrew


Re: Invalid value for shared scalar

2004-08-27 Thread chromatic
On Fri, 2004-08-27 at 11:03, Andrew Pimlott wrote:

> And most tests, even of threaded code, will call T::B from a single thread
> anyhow.

How do you know what tests people will write?  How do you know where
people will run those tests?

> There are two ways to do the compromise:
> 
> 1.  Go thread safe in T::B if threads has already been used, which just
> requires the programmer to use threads (or use a library that uses
> threads) before T::B--which he probably would do anyway.

  use_ok( 'Some::Module::Requiring::Threads' );

> 2.  Go thread safe in T::B only if the programmer requests it
> explicitly.  I would favor this, because I still see perl threads as
> experimental.

It doesn't matter how *you* see Perl threads if users who may run your
tests see them as worth using -- everyone using ActiveState's Perl for
example, or all of Red Hat's users (I believe), use threaded Perls.

How does the programmer know the characteristics of all of the Perl
installations on which the test will run?  How does he know the current
and future internals of all supporting modules and whether they use
threads or not?

 If you can't know all of this, then you have two options:

1) Ignore thread-safety altogether always.
2) Try to be thread-safe always.

Test::Builder takes the second approach.

-- c



Re: Invalid value for shared scalar

2004-08-27 Thread Andrew Pimlott
On Fri, Aug 27, 2004 at 09:20:09AM -0700, chromatic wrote:
> If you don't have an ithreads-capable Perl, T::B won't use threads.
> 
> If you do, it must,

Ideally, I would agree.  But I think a compromise is in order, because
perl threads aren't that mature and are error-prone to program.  And
most tests, even of threaded code, will call T::B from a single thread
anyhow.

There are two ways to do the compromise:

1.  Go thread safe in T::B if threads has already been used, which just
requires the programmer to use threads (or use a library that uses
threads) before T::B--which he probably would do anyway.

2.  Go thread safe in T::B only if the programmer requests it
explicitly.  I would favor this, because I still see perl threads as
experimental.

I think either of these is a reasonable trade-off.

Andrew

[1] http://www.nntp.perl.org/group/perl.perl5.porters/65583


Re: Invalid value for shared scalar

2004-08-27 Thread Andrew Pimlott
On Fri, Aug 27, 2004 at 10:16:48AM +0200, Rafael Garcia-Suarez wrote:
> Andrew Pimlott wrote:
> > 
> > Can you tell me where this limitation in perl threads is documented?
> > Is there any hope that it will be removed in the future?
> 
> It's not a limitation, if you share a hash it looks normal to me
> that you should share its elements too. (or you end up with weird
> quantum hashes that don't look the same from different threads...)

I would have expected the referent of a value assigned to a shared
scalar would be automatically shared (ie, promoted to shared if it is
unshared).  It seems like that is what the programmer is probably asking
for.

> That said, threads are underdocumented and the error message could
> be made much clearer.

Something like "reference to an unshared value assigned to a shared
scalar"?  Does that cover all the cases?

Andrew


Re: Invalid value for shared scalar

2004-08-27 Thread chromatic
On Fri, 2004-08-27 at 01:09, Andrew Pimlott wrote:

> I would still prefer that Test::Builder not use threads if I don't ask
> for it.

if( $] >= 5.008 && $Config{useithreads} ) {
require threads;
require threads::shared;
threads::shared->import;
}

If you don't have an ithreads-capable Perl, T::B won't use threads.

If you do, it must, because it has global data to which all threads need
access.  T::B can't know how people will call it, so it takes the
approach of being safe and at least attempting to work on both threaded
and non-threaded Perls.

Requiring a compile-time flag isn't a solution because you don't know
*how* other people will use your code, the characteristics of their
installations, or what other modules they will have installed.

-- c



Re: Invalid value for shared scalar

2004-08-27 Thread Rafael Garcia-Suarez
Andrew Pimlott wrote:
> 
> Can you tell me where this limitation in perl threads is documented?
> Is there any hope that it will be removed in the future?

It's not a limitation, if you share a hash it looks normal to me
that you should share its elements too. (or you end up with weird
quantum hashes that don't look the same from different threads...)

That said, threads are underdocumented and the error message could
be made much clearer.


Re: Invalid value for shared scalar

2004-08-27 Thread Andrew Pimlott
On Fri, Aug 27, 2004 at 09:24:20AM +0200, Rafael Garcia-Suarez wrote:
> In this case that's a bug in Test::Builder, not in perl.

Sure, but it is a very easy bug to make.  For perfectly normal code to
break with threads is scary to me.  And the fix looks fragile,
especially without a comment.  I would still prefer that Test::Builder
not use threads if I don't ask for it.

Can you tell me where this limitation in perl threads is documented?
Is there any hope that it will be removed in the future?

Andrew


Re: Invalid value for shared scalar

2004-08-27 Thread Rafael Garcia-Suarez
Andrew Pimlott wrote:
> I got this error, which I traced down to accidentally calling is() with
> a hashref as the third argument, where the name should have been:
> 
> use Test::More 'no_plan';
> is(1,1,{});

Autrijus has fixed this bug in bleadperl, see the patch at

http://public.activestate.com/cgi-bin/perlbrowse?patch=23167

It should be integrated in the next Test::Builder CPAN release.

> I found some mailing list discussions about this problem without much
> conclusion.  This leads me to believe that perl threads are not mature
> enough to foist on people without their express consent.  Of course,
> this case can be fixed, but if such innocent code can be broken by
> threads, I'm sure there are more bugs lurking.  And they are very
> frustrating to debug for people without experience in perl threads.

In this case that's a bug in Test::Builder, not in perl.

> So I suggest that Test::Builder not enable threads itself, unless
> explicitly requested.  Even if your code uses threads, it seems unlikely
> that you'd want to run your tests in parallel anyway.  You could perhaps
> enable thread-safe tests with
> 
> use Test::Builder 'thread_safe';


Invalid value for shared scalar

2004-08-26 Thread Andrew Pimlott
I got this error, which I traced down to accidentally calling is() with
a hashref as the third argument, where the name should have been:

use Test::More 'no_plan';
is(1,1,{});

This happens because Test::Builder tries to assign the name to a key of
a shared hash.  This simple case demonstrates the failure:

use threads;
use threads::shared;

share(%foo);
$foo{bar} = {};

I found some mailing list discussions about this problem without much
conclusion.  This leads me to believe that perl threads are not mature
enough to foist on people without their express consent.  Of course,
this case can be fixed, but if such innocent code can be broken by
threads, I'm sure there are more bugs lurking.  And they are very
frustrating to debug for people without experience in perl threads.

So I suggest that Test::Builder not enable threads itself, unless
explicitly requested.  Even if your code uses threads, it seems unlikely
that you'd want to run your tests in parallel anyway.  You could perhaps
enable thread-safe tests with

use Test::Builder 'thread_safe';

Andrew