[Boost-commit] svn:boost r44168 - in trunk: boost/thread/win32 libs/thread/test

From: anthony_at_[hidden]
Date: 2008-04-11 04:52:11

Author: anthonyw
Date: 2008-04-11 04:52:09 EDT (Fri, 11 Apr 2008)
New Revision: 44168
URL: http://svn.boost.org/trac/boost/changeset/44168

Log:
Added test and fix for win32 condition_variable broadcast bug similar to #1803
Text files modified: 
   trunk/boost/thread/win32/condition_variable.hpp      |    38 ++++++++++++++++++++++-------------     
   trunk/libs/thread/test/test_condition_notify_all.cpp |    42 ++++++++++++++++++++++++++++++++++++++++
   2 files changed, 66 insertions(+), 14 deletions(-)

Modified: trunk/boost/thread/win32/condition_variable.hpp
==============================================================================
--- trunk/boost/thread/win32/condition_variable.hpp	(original)
+++ trunk/boost/thread/win32/condition_variable.hpp	2008-04-11 04:52:09 EDT (Fri, 11 Apr 2008)
@@ -35,9 +35,9 @@
                     semaphore(0),count(0),notified(0)
                 {}
 
-                void release(unsigned count=1)
+                void release(unsigned count_to_release=1)
                 {
-                    detail::win32::ReleaseSemaphore(semaphore,count,0);
+                    detail::win32::ReleaseSemaphore(semaphore,count_to_release,0);
                 }
                 
             };
@@ -47,6 +47,13 @@
             list_entry generations[generation_count];
             detail::win32::handle wake_sem;
 
+            void wake_waiters(long count_to_wake)
+            {
+                detail::interlocked_write_release(&total_count,total_count-count_to_wake);
+                detail::win32::ReleaseSemaphore(wake_sem,count_to_wake,0);
+            }
+            
+
             static bool no_waiters(list_entry const& entry)
             {
                 return entry.count==0;
@@ -57,7 +64,7 @@
                 list_entry* const last_active_entry=std::remove_if(generations,generations+generation_count,no_waiters);
                 if(last_active_entry==generations+generation_count)
                 {
-                    broadcast_entry(generations[generation_count-1],false);
+                    broadcast_entry(generations[generation_count-1]);
                 }
                 else
                 {
@@ -75,15 +82,9 @@
                 generations[0]=list_entry();
             }
 
-            void broadcast_entry(list_entry& entry,bool wake)
+            void broadcast_entry(list_entry& entry)
             {
-                long const count_to_wake=entry.count;
-                detail::interlocked_write_release(&total_count,total_count-count_to_wake);
-                if(wake)
-                {
-                    detail::win32::ReleaseSemaphore(wake_sem,count_to_wake,0);
-                }
-                entry.release(count_to_wake);
+                entry.release(entry.count);
                 entry.count=0;
                 dispose_entry(entry);
             }
@@ -237,8 +238,7 @@
                     {
                         return;
                     }
-                    detail::win32::ReleaseSemaphore(wake_sem,1,0);
-                    detail::interlocked_write_release(&total_count,total_count-1);
+                    wake_waiters(1);
                     
                     unsigned waiting_count=0;
                     
@@ -262,6 +262,7 @@
                     }
                     if(waiting_count<=total_count)
                     {
+                        shift_generations_down();
                         ensure_generation_present();
                         generations[0].release();
                     }
@@ -273,14 +274,23 @@
                 if(detail::interlocked_read_acquire(&total_count))
                 {
                     boost::mutex::scoped_lock internal_lock(internal_mutex);
+                    long waiting_count=total_count;
+                    
+                    wake_waiters(total_count);
                     for(unsigned generation=active_generation_count;generation!=0;--generation)
                     {
                         list_entry& entry=generations[generation-1];
                         if(entry.count)
                         {
-                            broadcast_entry(entry,true);
+                            waiting_count-=entry.count;
+                            broadcast_entry(entry);
                         }
                     }
+                    if(waiting_count)
+                    {
+                        ensure_generation_present();
+                        generations[0].release(waiting_count);
+                    }
                     active_generation_count=0;
                 }
             }

Modified: trunk/libs/thread/test/test_condition_notify_all.cpp
==============================================================================
--- trunk/libs/thread/test/test_condition_notify_all.cpp	(original)
+++ trunk/libs/thread/test/test_condition_notify_all.cpp	2008-04-11 04:52:09 EDT (Fri, 11 Apr 2008)
@@ -159,6 +159,47 @@
     }
 }
 
+namespace
+{
+    boost::mutex multiple_wake_mutex;
+    boost::condition_variable multiple_wake_cond;
+    unsigned multiple_wake_count=0;
+
+    void wait_for_condvar_and_increase_count()
+    {
+        boost::mutex::scoped_lock lk(multiple_wake_mutex);
+        multiple_wake_cond.wait(lk);
+        ++multiple_wake_count;
+    }
+    
+}
+
+
+void do_test_notify_all_following_notify_one_wakes_all_threads()
+{
+    boost::thread thread1(wait_for_condvar_and_increase_count);
+    boost::thread thread2(wait_for_condvar_and_increase_count);
+
+    boost::this_thread::sleep(boost::posix_time::milliseconds(200));
+    multiple_wake_cond.notify_one();
+
+    boost::thread thread3(wait_for_condvar_and_increase_count);
+
+    boost::this_thread::sleep(boost::posix_time::milliseconds(200));
+    multiple_wake_cond.notify_one();
+    multiple_wake_cond.notify_all();
+    boost::this_thread::sleep(boost::posix_time::milliseconds(200));
+    
+    {
+        boost::mutex::scoped_lock lk(multiple_wake_mutex);
+        BOOST_CHECK(multiple_wake_count==3);
+    }
+
+    thread1.join();
+    thread2.join();
+    thread3.join();
+}
+
 void test_condition_notify_all()
 {
     timed_test(&do_test_condition_notify_all_wakes_from_wait, timeout_seconds);
@@ -166,6 +207,7 @@
     timed_test(&do_test_condition_notify_all_wakes_from_timed_wait, timeout_seconds);
     timed_test(&do_test_condition_notify_all_wakes_from_timed_wait_with_predicate, timeout_seconds);
     timed_test(&do_test_condition_notify_all_wakes_from_relative_timed_wait_with_predicate, timeout_seconds);
+    timed_test(&do_test_notify_all_following_notify_one_wakes_all_threads, timeout_seconds);
 }