revised warnings + more tests + docs

Message-ID: <20020518222451.E7275@fdgroup.com>

p4raw-id: //depot/perl@16685

4 files changed, 99 insertions, 56 deletions

diff --git a/ext/threads/shared/shared.pm b/ext/threads/shared/shared.pm
index 4ffe261a93..7536495437 100644
--- a/ext/threads/shared/shared.pm
+++ b/ext/threads/shared/shared.pm
@@ -72,6 +72,8 @@ threads::shared - Perl extension for sharing data structures between threads
   use threads;
   use threads::shared;
 
+  my $var : shared;
+
   my($scalar, @array, %hash);
   share($scalar);
   share(@array);
@@ -79,21 +81,22 @@ threads::shared - Perl extension for sharing data structures between threads
   my $bar = share([]);
   $hash{bar} = share({});
 
-  lock(%hash);
-  unlock(%hash);
+  { lock(%hash); ...  }
+
   cond_wait($scalar);
   cond_broadcast(@array);
   cond_signal(%hash);
 
 =head1 DESCRIPTION
 
-This modules allows you to share() variables. These variables will
-then be shared across different threads (and pseudoforks on
-win32). They are used together with the threads module.
+By default, variables are private to each thread, and each newly created
+thread gets a private copy of each existing variable.  This module allows
+you to share variables across different threads (and pseudoforks on
+win32). It is used together with the threads module.
 
 =head1 EXPORT
 
-C<share>, C<lock>, C<unlock>, C<cond_wait>, C<cond_signal>, C<cond_broadcast>
+C<share>, C<lock>, C<cond_wait>, C<cond_signal>, C<cond_broadcast>
 
 =head1 FUNCTIONS
 
@@ -107,14 +110,16 @@ hash, scalar ref, array ref or hash ref. C<share> will return the shared value.
 C<share> will traverse up references exactly I<one> level.
 C<share(\$a)> is equivalent to C<share($a)>, while C<share(\\$a)> is not.
 
+A variable can also be marked as shared at compile time by using the
+C<shared> attribute: C<my $var : shared>.
+
 =item lock VARIABLE
 
 C<lock> places a lock on a variable until the lock goes out of scope.  If
 the variable is locked by another thread, the C<lock> call will block until
 it's available. C<lock> is recursive, so multiple calls to C<lock> are
 safe -- the variable will remain locked until the outermost lock on the
-variable goes out of scope or C<unlock> is called enough times to match
-the number of calls to <lock>.
+variable goes out of scope.
 
 If a container object, such as a hash or array, is locked, all the elements
 of that container are not locked. For example, if a thread does a C<lock
@@ -123,15 +128,9 @@ of that container are not locked. For example, if a thread does a C<lock
 C<lock> will traverse up references exactly I<one> level.
 C<lock(\$a)> is equivalent to C<lock($a)>, while C<lock(\\$a)> is not.
 
-
-=item unlock VARIABLE
-
-C<unlock> takes a B<locked> variable and decrements the lock count.
-If the lock count is zero the variable is unlocked. It is not necessary
-to call C<unlock> but it can be useful to reduce lock contention.
-
-C<unlock> will traverse up references exactly I<one> level.
-C<unlock(\$a)> is equivalent to C<unlock($a)>, while C<unlock(\\$a)> is not.
+Note that you cannot explicitly unlock a variable; you can only wait for
+the lock to go out of scope. If you need more fine-grained control, see
+L<threads::shared::semaphore>.
 
 =item cond_wait VARIABLE
 
@@ -141,8 +140,10 @@ or C<cond_broadcast> for that same locked variable. The variable that
 C<cond_wait> blocked on is relocked after the C<cond_wait> is satisfied.
 If there are multiple threads C<cond_wait>ing on the same variable, all but
 one will reblock waiting to reacquire the lock on the variable. (So if
-you're only using C<cond_wait> for synchronization, give up the lock as
-soon as possible)
+you're only using C<cond_wait> for synchronisation, give up the lock as
+soon as possible). The two actions of unlocking the variable and entering
+the blocked wait state are atomic, The two actions of exiting from the
+blocked wait state and relocking the variable are not.
 
 It is important to note that the variable can be notified even if no
 thread C<cond_signal> or C<cond_broadcast> on the variable. It is therefore
@@ -157,7 +158,14 @@ one thread is blocked in a C<cond_wait> on that variable, only one (and
 which one is indeterminate) will be unblocked.
 
 If there are no threads blocked in a C<cond_wait> on the variable, the
-signal is discarded.
+signal is discarded. By always locking before signaling, you can (with
+care), avoid signaling before another thread has entered cond_wait().
+
+C<cond_signal> will normally generate a warning if you attempt to use it
+on an unlocked variable. On the rare occasions where doing this may be
+sensible, you can skip the warning with
+
+    { no warnings 'threads'; cond_signal($foo) }
 
 =item cond_broadcast VARIABLE
 
diff --git a/ext/threads/shared/shared.xs b/ext/threads/shared/shared.xs
index 9b0ca50c66..14524f6dda 100644
--- a/ext/threads/shared/shared.xs
+++ b/ext/threads/shared/shared.xs
@@ -732,7 +732,7 @@ Perl_sharedsv_locksv(pTHX_ SV *sv)
 
     if(SvROK(sv))
 	sv = SvRV(sv);
-    shared = Perl_sharedsv_find(aTHX, sv);
+    shared = Perl_sharedsv_find(aTHX_ sv);
     if(!shared)
        croak("lock can only be used on shared values");
     Perl_sharedsv_lock(aTHX_ shared);
@@ -962,7 +962,7 @@ share(SV *ref)
 	ref = SvRV(ref);
 	if(SvROK(ref))
 	    ref = SvRV(ref);
-	Perl_sharedsv_share(aTHX, ref);
+	Perl_sharedsv_share(aTHX_ ref);
 
 void
 lock_enabled(SV *ref)
@@ -972,7 +972,7 @@ lock_enabled(SV *ref)
 	ref = SvRV(ref);
 	if(SvROK(ref))
 	    ref = SvRV(ref);
-	shared = Perl_sharedsv_find(aTHX, ref);
+	shared = Perl_sharedsv_find(aTHX_ ref);
 	if(!shared)
 	   croak("lock can only be used on shared values");
 	Perl_sharedsv_lock(aTHX_ shared);
@@ -1017,6 +1017,9 @@ cond_signal_enabled(SV *ref)
 	if(SvROK(ref))
 	    ref = SvRV(ref);
 	shared = Perl_sharedsv_find(aTHX_ ref);
+	if (ckWARN(WARN_THREADS) && shared->lock.owner != aTHX)
+	    Perl_warner(aTHX_ packWARN(WARN_THREADS),
+			    "cond_signal() called on unlocked variable");
 	if(!shared)
 	    croak("cond_signal can only be used on shared values");
 	COND_SIGNAL(&shared->user_cond);
@@ -1032,6 +1035,9 @@ cond_broadcast_enabled(SV *ref)
 	shared = Perl_sharedsv_find(aTHX_ ref);
 	if(!shared)
 	    croak("cond_broadcast can only be used on shared values");
+	if (ckWARN(WARN_THREADS) && shared->lock.owner != aTHX)
+	    Perl_warner(aTHX_ packWARN(WARN_THREADS),
+			    "cond_broadcast() called on unlocked variable");
 	COND_BROADCAST(&shared->user_cond);
 
 #endif /* USE_ITHREADS */
diff --git a/ext/threads/t/thread.t b/ext/threads/t/thread.t
index 435f3bd919..9a2bb28b7e 100644
--- a/ext/threads/t/thread.t
+++ b/ext/threads/t/thread.t
@@ -12,7 +12,7 @@ BEGIN {
 
 use ExtUtils::testlib;
 use strict;
-BEGIN { $| = 1; print "1..21\n" };
+BEGIN { $| = 1; print "1..24\n" };
 use threads;
 use threads::shared;
 
@@ -121,3 +121,23 @@ sub threaded {
     ok($thr6->join());
     ok($thr7->join());
 }
+
+# test that 'yield' is importable
+
+package Test1;
+
+use threads 'yield';
+yield;
+main::ok(1);
+
+package main;
+
+
+# test async
+
+{
+    my $th = async {return 1 };
+    ok($th);
+    ok($th->join());
+}
+
diff --git a/ext/threads/threads.pm b/ext/threads/threads.pm
index d74e85fac3..43d1f0a01f 100755
--- a/ext/threads/threads.pm
+++ b/ext/threads/threads.pm
@@ -83,30 +83,28 @@ threads - Perl extension allowing use of interpreter based threads from perl
 
 =head1 SYNOPSIS
 
-use threads;
+    use threads;
 
-sub start_thread {
-    print "Thread started\n";
-}
-
-my $thread = threads->create("start_thread","argument");
-
-$thread->create(sub { print "I am a thread"},"argument");
-
-$thread->join();
+    sub start_thread {
+	print "Thread started\n";
+    }
 
-$thread->detach();
+    my $thread  = threads->create("start_thread","argument");
+    my $thread2 = $thread->create(sub { print "I am a thread"},"argument");
+    my $thread3 = async { foreach (@files) { ... } };
 
-$thread = threads->self();
+    $thread->join();
+    $thread->detach();
 
-threads->tid();
-threads->self->tid();
+    $thread = threads->self();
 
-$thread->tid();
+    $thread->tid();
+    threads->tid();
+    threads->self->tid();
 
-threads->yield();
+    threads->yield();
 
-threads->list();
+    threads->list();
 
 =head1 DESCRIPTION
 
@@ -123,7 +121,7 @@ important to note that variables are not shared between threads, all
 variables are per default thread local.  To use shared variables one
 must use threads::shared.
 
-It is also important to note that you preferably enable threads by
+It is also important to note that you must enable threads by
 doing C<use threads> as early as possible and that it is not possible
 to enable threading inside an eval "";  In particular, if you are
 intending to share variables with threads::shared, you must
@@ -136,32 +134,43 @@ a warning if you do it the other way around.
 
 This will create a new thread with the entry point function and give
 it LIST as parameters.  It will return the corresponding threads
-object.
+object. The new() method is an alias for create().
 
 =item $thread->join
 
-This will wait for the corresponding thread to join. When it finishes
-join will return the return values of the entry point function.  If a
-thread has been detached, an error will be thrown..
+This will wait for the corresponding thread to join. When the thread finishes,
+join() will return the return values of the entry point function. If the
+thread has been detached, an error will be thrown. If the program
+exits without all other threads having been either joined or detached,
+then a warning will be issued. (A program exits either because one of its
+threads explicitly calls exit(), or in the case of the main thread, reaches
+the end of the main program file.)
 
 =item $thread->detach
 
-Will throw away the return value from the thread and make it
-non-joinable.
+Will make the thread unjoinable, and cause any eventual return value to be
+discarded.
 
 =item threads->self
 
-This will return the object for the current thread.
+This will return the thread object for the current thread.
 
 =item $thread->tid
 
-This will return the id of the thread.  threads->tid() is a quick way 
-to get current thread id if you don't have your thread handy.
+This will return the id of the thread.  Thread IDs are integers, with the
+main thread in a program being 0.  Currently Perl assigns a unique tid to
+every thread ever created in your program, assigning the first thread to
+be created a tid of 1, and increasing the tid by 1 for each new thread
+that's created.
+
+NB the class method C<< threads->tid() >> is a quick way to get the
+current thread id if you don't have your thread object handy.
 
 =item threads->yield();
 
-This will tell the OS to let this thread yield CPU time to other threads.
-However this is highly depending on the underlying thread implementation.
+This is a suggestion to the OS to let this thread yield CPU time to other
+threads.  What actually happens is highly dependent upon the underlying
+thread implementation.
 
 You may do C<use threads qw(yield)> then use just a bare C<yield> in your
 code.
@@ -174,7 +183,7 @@ This will return a list of all non joined, non detached threads.
 
 C<async> creates a thread to execute the block immediately following
 it.  This block is treated as an anonymous sub, and so must have a
-semi-colon after the closing brace. Like C<threads-&gt;new>, C<async>
+semi-colon after the closing brace. Like C<< threads->new >>, C<async>
 returns a thread object.
 
 =back
@@ -194,11 +203,11 @@ exit from then main thread.
 
 =head1 BUGS / TODO
 
-The current implmentation of threads has been an attempt to get
+The current implementation of threads has been an attempt to get
 a correct threading system working that could be built on, 
 and optimized, in newer versions of perl.
 
-Current the overhead of creating a thread is rather large,
+Currently the overhead of creating a thread is rather large,
 also the cost of returning values can be large. These are areas
 were there most likely will be work done to optimize what data
 that needs to be cloned.