ChangeLogTag:Sat Mar 15 10:55:01 2003 Douglas C. Schmidt <schmidt@macarena.cs.wustl.edu>

9 files changed, 234 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index 3f7164b5f0d..27194ad9564 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,32 @@
+Sat Mar 15 10:55:01 2003  Douglas C. Schmidt  <schmidt@macarena.cs.wustl.edu>
+
+	* tests/Makefile:
+	* tests/Makefile.bor:
+	* tests/run_test.lst: Added Recursive_Condition_Bug_Test.cpp to
+	  these files.
+
+	* tests: Added a new Recursive_Condition_Bug_Test.cpp to check
+	  that the fix from Leonid Kvetnyi is correct and doesn't cause
+	  other problems.  
+
+	* tests/Recursive_Condition_Test.cpp (ACE_TMAIN): Minor reformatting.
+
+	* ace/OS.i (recursive_mutex_cond_unlock): Added another check for
+	  m->RecursionCount > 1 to fix problems where the test program hangs.
+	  Thanks to Leonid Kvetnyi <leonidk@nice.com> for this fix.
+
+	* ace/Event_Handler_T.h: Changed
+  
+          typedef int (T::*SIG_HANDLER) (ACE_HANDLE, siginfo_t*, ucontext_t*);
+
+	  to this
+
+	  typedef int (T::*SIG_HANDLER) (int, siginfo_t*, ucontext_t*);
+  
+          so that we're consistent across platforms where ACE_HANDLE is a
+ 	  HANDLE, not an int!  Thanks to Johnny Willemsen
+ 	  <jwillemsen@remedy.nl> for reporting this.
+
 Fri Mar 14 22:26:56 2003  Nanbor Wang  <nanbor@cs.wustl.edu>
 
 	* bin/MakeProjectCreator/config/ciao_client.mpb:
diff --git a/THANKS b/THANKS
index fbef00f0ec9..1415abf121b 100644
--- a/THANKS
+++ b/THANKS
@@ -1676,6 +1676,7 @@ Martin Brown <mpatalberta@yahoo.com>
 Terry Mihm <terrym@firstlogic.com>
 Jeff Gray <gray@cis.uab.edu>
 Rob Eger <reger@txcorp.com>
+Leonid Kvetnyi <leonidk@nice.com>
 
 I would particularly like to thank Paul Stephenson, who worked with me
 at Ericsson in the early 1990's.  Paul devised the recursive Makefile
diff --git a/ace/Event_Handler_T.h b/ace/Event_Handler_T.h
index c6400f93281..7dfc34f4ee6 100644
--- a/ace/Event_Handler_T.h
+++ b/ace/Event_Handler_T.h
@@ -78,7 +78,7 @@ public:
   typedef int (T::*CL_HANDLER) (ACE_HANDLE, ACE_Reactor_Mask);
 
   /// = Initialization and termination methods.
-  typedef int (T::*SIG_HANDLER) (ACE_HANDLE, siginfo_t*, ucontext_t*);
+  typedef int (T::*SIG_HANDLER) (int, siginfo_t*, ucontext_t*);
 
   /// Initialize the op_handler.
   ACE_Event_Handler_T (T *op_handler,
diff --git a/ace/OS.i b/ace/OS.i
index e617e4b7b92..4b8cf137c92 100644
--- a/ace/OS.i
+++ b/ace/OS.i
@@ -2628,7 +2628,7 @@ ACE_OS::recursive_mutex_cond_unlock (ACE_recursive_thread_mutex_t *m,
   // it. Remember how many times, and reacquire it that many more times when
   // the condition is signaled.
   state.relock_count_ = 0;
-  while (m->LockCount > 0)
+  while (m->LockCount > 0 && m->RecusionCount > 1)
     {
       // This may fail if the current thread doesn't own the mutex. If it
       // does fail, it'll be on the first try, so don't worry about resetting
diff --git a/tests/Makefile b/tests/Makefile
index e2f577021c2..0943648d86d 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -90,6 +90,7 @@ BIN =   ACE_Test \
         Reactor_Timer_Test \
         Reader_Writer_Test \
         Recursive_Condition_Test \
+        Recursive_Condition_Bug_Test \
         Recursive_Mutex_Test \
 	Refcounted_Auto_Ptr_Test \
         Reverse_Lock_Test \
diff --git a/tests/Makefile.bor b/tests/Makefile.bor
index 7c8cb325366..c7ea677ec08 100644
--- a/tests/Makefile.bor
+++ b/tests/Makefile.bor
@@ -88,6 +88,7 @@ NAMES = \
 	Reactors_Test \
 	Reader_Writer_Test \
 	Recursive_Condition_Test \
+	Recursive_Condition_Bug_Test \
 	Recursive_Mutex_Test \
         Refcounted_Auto_Ptr_Test \
         Reverse_Lock_Test \
diff --git a/tests/Recursive_Condition_Bug_Test.cpp b/tests/Recursive_Condition_Bug_Test.cpp
new file mode 100644
index 00000000000..16371681bb3
--- /dev/null
+++ b/tests/Recursive_Condition_Bug_Test.cpp
@@ -0,0 +1,181 @@
+// $Id$
+
+// ============================================================================
+//
+// = LIBRARY
+//    tests
+//
+// = FILENAME
+//    Recursive_Condition_Bug_Test.cpp
+//
+// = DESCRIPTION
+//      This test program validates the functionality of the
+//      ACE_Condition<ACE_Recursive_Thread_Mutex> template
+//      specialization when combined with the
+//      ACE_Thread_Timer_Queue_Adapter on Win32 and Posix pthreads.
+//      It was added to test for bugs with the ACE_OS recursive
+//      condition implementation.  
+//
+// = AUTHOR
+//    Leonid Kvetnyi <leonidk@nice.com> and
+//    Douglas C. Schmidt <schmidt@cs.wustl.edu>
+//
+// ============================================================================
+
+#include "test_config.h"
+#include "ace/Task_T.h"
+#include "ace/Activation_Queue.h"
+#include "ace/Timer_Heap.h"
+#include "ace/Timer_Queue_Adapters.h"
+
+ACE_RCSID(tests, Recursive_Condition_Bug_Test, "$Id$")
+
+// Number of iterations for the performance tests. 
+static int max_iterations = 30;
+
+#if defined (ACE_HAS_THREADS)
+
+typedef ACE_Thread_Timer_Queue_Adapter<ACE_Timer_Heap> Thread_Timer_Queue;
+
+class Test_Handler;
+
+class Test_Task : public ACE_Task<ACE_MT_SYNCH>
+{
+public:
+  virtual int svc (void)
+  {
+    while (--max_iterations > 0)
+      {
+        // dequeue the next object
+        ACE_Message_Block * mb;
+
+        if (this->getq (mb) == -1)
+          ACE_ERROR_RETURN ((LM_ERROR,
+                             ACE_TEXT ("(%t) %p\n"),
+                             ACE_TEXT ("getq failed")),
+                            -1);
+
+        Test_Handler *handler = ACE_reinterpret_cast (Test_Handler *, mb->base ());
+        mb->release ();
+
+        ACE_Time_Value timeout = ACE_OS::gettimeofday () + ACE_Time_Value (1, 0);
+
+        if (timer_queue_.schedule (ACE_reinterpret_cast (ACE_Event_Handler *,
+                                                         handler),
+                                   this,
+                                   timeout) == -1)
+          ACE_ERROR_RETURN ((LM_ERROR,
+                             ACE_TEXT ("(%t) %p\n"),
+                             ACE_TEXT ("schedule failed")),
+                            -1);
+
+        ACE_DEBUG ((LM_INFO, 
+                    ACE_TEXT ("(%t) %d iteration(s) remain\n"),
+                    max_iterations));
+      }
+
+    timer_queue_.deactivate ();
+    ACE_DEBUG ((LM_INFO,
+                ACE_TEXT ("(%t) leaving the Test_task\n")));
+    return 0;
+  }
+
+  virtual int open (void * = 0)
+  {
+    if (ACE_Task<ACE_MT_SYNCH>::activate (THR_NEW_LWP, 1) != 0)
+      ACE_ERROR_RETURN ((LM_ERROR,
+                         ACE_TEXT ("(%t) %p\n"),
+                         ACE_TEXT ("Test_Task::activate")),
+                        -1);
+    if (0 != timer_queue_.activate ())
+      ACE_ERROR_RETURN ((LM_ERROR,
+                         ACE_TEXT ("(%t) %p\n"),
+                         ACE_TEXT ("Test_Task::queue activate")),
+                        -1);
+    return 0;
+  }
+
+private:
+  Thread_Timer_Queue timer_queue_;
+};
+
+class Test_Handler : public ACE_Event_Handler
+{
+public:
+  virtual int handle_timeout (const ACE_Time_Value &,
+                              const void *arg)
+  {
+    ACE_DEBUG ((LM_DEBUG,
+                ACE_TEXT ("(%t) Test_Handler::handle_timeout\n")));
+
+    void *nc_arg = ACE_const_cast (void *, arg);
+    Test_Task *test_task =
+      ACE_reinterpret_cast (Test_Task *, nc_arg);
+    ACE_Message_Block *mb;
+    ACE_NEW_MALLOC_RETURN (mb,
+                           ACE_static_cast (ACE_Message_Block *,
+                                            ACE_Allocator::instance()->malloc (sizeof (ACE_Message_Block))),
+                           ACE_Message_Block (sizeof (*this),    // size
+                                              ACE_Message_Block::MB_DATA, // type
+                                              0,       // cont
+                                              (char *) this),     // data
+                           -1);
+
+    test_task->putq (mb);
+    return 0;
+  }
+};
+
+#endif /* ACE_HAS_THREADS */
+
+int
+ACE_TMAIN (int, ACE_TCHAR *[])
+{
+  ACE_START_TEST (ACE_TEXT ("Recursive_Condition_Bug_Test"));
+
+#if defined (ACE_HAS_THREADS)
+
+  // Timer queue usage.
+
+  Test_Handler handler;
+  Test_Task task;
+
+  if (0 != task.open ())
+    ACE_ERROR_RETURN ((LM_ERROR,
+                       ACE_TEXT ("(%t) %p\n"),
+                       ACE_TEXT ("open")),
+                      -1);
+
+  ACE_Message_Block *mb;
+  ACE_NEW_MALLOC_RETURN (mb,
+                         ACE_static_cast(ACE_Message_Block *,
+                                         ACE_Allocator::instance()->malloc (sizeof (ACE_Message_Block))),
+                         ACE_Message_Block (sizeof (handler),    // size
+                                            ACE_Message_Block::MB_DATA, // type
+                                            0,       // cont
+                                            (char *) &handler),     // data
+                         -1);
+
+  if (-1 == task.putq (mb))
+    ACE_ERROR_RETURN ((LM_ERROR,
+                       ACE_TEXT ("(%t) %p\n"), 
+                       ACE_TEXT ("putq")),
+                      -1);
+
+  if (ACE_Thread_Manager::instance ()->wait () == -1)
+    ACE_ERROR_RETURN ((LM_ERROR,
+                       "wait on Thread_Manager failed"),
+                      -1);
+#else
+  ACE_ERROR ((LM_ERROR,
+              ACE_TEXT ("ACE doesn't support recursive condition variables on this platform\n")));
+#endif /* ACE_HAS_THREADS */
+  ACE_END_TEST;
+  return 0;
+}
+
+#if defined (ACE_HAS_EXPLICIT_TEMPLATE_INSTANTIATION)
+template class ACE_Thread_Timer_Queue_Adapter<ACE_Timer_Heap>;
+#elif defined (ACE_HAS_TEMPLATE_INSTANTIATION_PRAGMA)
+#pragma instantiate ACE_Thread_Timer_Queue_Adapter<ACE_Timer_Heap>
+#endif /* ACE_HAS_EXPLICIT_TEMPLATE_INSTANTIATION */
diff --git a/tests/Recursive_Condition_Test.cpp b/tests/Recursive_Condition_Test.cpp
index 7f6b0cea25b..09e28d72d6d 100644
--- a/tests/Recursive_Condition_Test.cpp
+++ b/tests/Recursive_Condition_Test.cpp
@@ -9,7 +9,7 @@
 //    Recursive_Condition_Test.cpp
 //
 // = DESCRIPTION
-//      This test program verifies the functionality of the
+//      This test program validates the functionality of the
 //      ACE_Condition<ACE_Recursive_Thread_Mutex> template
 //      specialization when combined with the
 //      ACE_Thread_Timer_Queue_Adapter on Win32 and Posix pthreads.
@@ -32,12 +32,12 @@ ACE_RCSID(tests, Recursive_Condition_Test, "$Id$")
 
 #if defined (ACE_HAS_THREADS)
 
-  typedef ACE_Thread_Timer_Queue_Adapter<ACE_Timer_Heap> Thread_Timer_Queue;
+typedef ACE_Thread_Timer_Queue_Adapter<ACE_Timer_Heap> Thread_Timer_Queue;
 
 class Test_Handler : public ACE_Event_Handler
 {
 public:
-  Test_Handler () : nr_expirations_ (0) {}
+  Test_Handler (void) : nr_expirations_ (0) {}
   int nr_expirations (void) { return this->nr_expirations_; }
 
   virtual int handle_timeout (const ACE_Time_Value &,
@@ -71,7 +71,8 @@ ACE_SYNCH_RECURSIVE_CONDITION condition_(mutex_);
 // Test driver sets this to non-zero before spawning and to zero for waiter.
 int protected_int = 0;
 
-ACE_THR_FUNC_RETURN waiter (void *)
+static ACE_THR_FUNC_RETURN 
+waiter (void *)
 {
   if (mutex_.acquire () != 0)
     ACE_ERROR_RETURN ((LM_ERROR, ACE_TEXT ("(%t) %p\n"),
@@ -94,9 +95,8 @@ ACE_THR_FUNC_RETURN waiter (void *)
   return 0;
 }
 
-
-int
-test_1(void)
+static int
+test_1 (void)
 {
   protected_int = 1;
   if (ACE_Thread_Manager::instance()->spawn (waiter) == -1)
@@ -124,8 +124,8 @@ test_1(void)
   return 0;
 }
 
-int
-test_2(void)
+static int
+test_2 (void)
 {
   protected_int = 1;
   if (ACE_Thread_Manager::instance()->spawn (waiter) == -1)
@@ -154,8 +154,8 @@ test_2(void)
   return 0;
 }
 
-int
-test_3()
+static int
+test_3 (void)
 {
   protected_int = 1;
   if (ACE_Thread_Manager::instance()->spawn_n (4, waiter) == -1)
@@ -182,8 +182,8 @@ test_3()
   return 0;
 }
 
-int
-test_4()
+static int
+test_4 (void)
 {
   const int recurse_count = 3;
 
@@ -195,15 +195,11 @@ test_4()
   ACE_OS::sleep (2);
   int i;
   for (i = 0; i < recurse_count; ++i)
-    {
-      if (mutex_.acquire () == -1)
-        {
-          ACE_ERROR_RETURN ((LM_ERROR, ACE_TEXT ("pass %d, %p\n"),
-                             i + 1,
-                             ACE_TEXT ("recursive acquire")),
-                            1);
-        }
-    }
+    if (mutex_.acquire () == -1)
+      ACE_ERROR_RETURN ((LM_ERROR, ACE_TEXT ("pass %d, %p\n"),
+                         i + 1,
+                         ACE_TEXT ("recursive acquire")),
+                        1);
 
   if (mutex_.get_nesting_level () != i)
     ACE_ERROR_RETURN ((LM_ERROR, ACE_TEXT("test 4 nesting level %d;"),
diff --git a/tests/run_test.lst b/tests/run_test.lst
index 69d6e02392f..fa024343c03 100644
--- a/tests/run_test.lst
+++ b/tests/run_test.lst
@@ -98,6 +98,7 @@ Reactor_Performance_Test: !chorus
 Reactor_Timer_Test
 Reader_Writer_Test
 Recursive_Condition_Test: !ST
+Recursive_Condition_Bug_Test: !ST
 Recursive_Mutex_Test: !ST
 Refcounted_Auto_Ptr_Test: !VxWorks
 Reverse_Lock_Test