ChangeLogTag: Tue Jul 31 06:58:59 2001 Balachandran Natarajan <bala@cs.wustl.edu>

14 files changed, 118 insertions, 81 deletions

diff --git a/TAO/ChangeLogs/ChangeLog-02a b/TAO/ChangeLogs/ChangeLog-02a
index 216014a5b11..3ff090f9bb9 100644
--- a/TAO/ChangeLogs/ChangeLog-02a
+++ b/TAO/ChangeLogs/ChangeLog-02a
@@ -1,3 +1,38 @@
+Tue Jul 31 06:58:59 2001  Balachandran Natarajan  <bala@cs.wustl.edu>
+
+	This checkin is for fixing a race condition while trying to
+	manipulate the number of upcalls. This was not a problem before
+	575 fix, as the manipulation was done when there was an implicit
+	synchronisation in the TP Reactor. As the implicit synchronisation
+	has been broken, we had a race condition. The surpsising element
+	was the fact that it took sometime to figure out this race
+	condition. We have now added a lock that will be held by the
+	thread  before the variable is manipulated.
+	
+	* tao/Connection_Handler.cpp:
+	* tao/Connection_Handler.h:
+	* tao/ Connection_Handler.inl: Added a lock to the class. Also
+	  added three methods, incr_pending_upcalls (),
+	  decr_pending_upcalls () and pending_upcalls (). The first two
+	  does the manipulation of the pending_upcalls_ variable after
+	  holding the lock.  
+	
+	* tao/IIOP_Connection_Handler.cpp:
+	* tao/IIOP_Connection_Handler.h: Removed he peding_upcalls_
+	  variable and called the incr_pending_upcalls () and
+	  decr_pending_upcalls () to achieve what needs to be done.
+
+	* tao/Strategies/DIOP_Connection_Handler.cpp
+	* tao/Strategies/DIOP_Connection_Handler.h
+	* tao/Strategies/SHMIOP_Connection_Handler.cpp
+	* tao/Strategies/SHMIOP_Connection_Handler.h
+	* tao/Strategies/UIOP_Connection_Handler.cpp
+	* tao/Strategies/UIOP_Connection_Handler.h: 
+	* orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.h:
+	* orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.cpp:
+	  Replicated the changes from IIOP to the above protocols. 
+	
+
 Sun Jul 29 19:31:34 2001  Douglas C. Schmidt  <schmidt@tango.doc.wustl.edu>
 
 	* tao/RTCORBA/RT_Mutex.cpp (try_lock): Since we fixed the timed
diff --git a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.cpp b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.cpp
index c6a112d777b..b7f31d62156 100644
--- a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.cpp
+++ b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.cpp
@@ -30,7 +30,6 @@ TAO_SSLIOP_Connection_Handler::TAO_SSLIOP_Connection_Handler (
     TAO_Connection_Handler (0),
     current_ (),
     current_impl_ (),
-    pending_upcalls_ (1),
     tcp_properties_ (0),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
 {
@@ -51,7 +50,6 @@ TAO_SSLIOP_Connection_Handler::TAO_SSLIOP_Connection_Handler (
     TAO_Connection_Handler (orb_core),
     current_ (),
     current_impl_ (),
-    pending_upcalls_ (1),
     tcp_properties_ (ACE_static_cast
                      (TAO_IIOP_Properties *, arg)),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
@@ -214,8 +212,8 @@ TAO_SSLIOP_Connection_Handler::handle_close (ACE_HANDLE handle,
                  handle,
                  rm));
 
-  --this->pending_upcalls_;
-  if (this->pending_upcalls_ <= 0)
+  long pending = this->decr_pending_upcalls ();
+  if (pending <= 0)
     {
       if (this->transport ()->wait_strategy ()->is_registered ())
         {
@@ -349,7 +347,7 @@ int
 TAO_SSLIOP_Connection_Handler::handle_input (ACE_HANDLE)
 {
     // Increase the reference count on the upcall that have passed us.
-  this->pending_upcalls_++;
+  this->incr_pending_upcalls ();
 
   this->resume_flag_ = TAO_RESUMES_CONNECTION_HANDLER;
 
@@ -359,7 +357,7 @@ TAO_SSLIOP_Connection_Handler::handle_input (ACE_HANDLE)
   int retval = this->transport ()->handle_input_i (resume_handle);
 
   // The upcall is done. Bump down the reference count
-  if (--this->pending_upcalls_ <= 0)
+  if (this->decr_pending_upcalls () <= 0)
     retval = -1;
 
   if (retval == -1)
diff --git a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.h b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.h
index 6c07a118aef..a679dd0a707 100644
--- a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.h
+++ b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Connection_Handler.h
@@ -136,12 +136,6 @@ protected:
 
 private:
 
-  /// Reference count.It is used to count nested upcalls on this
-  /// svc_handler i.e., the connection can close during nested upcalls,
-  /// you should not delete the svc_handler until the stack unwinds
-  /// from the nested upcalls.
-  long pending_upcalls_;
-
   /// TCP configuration for this connection.
   TAO_IIOP_Properties *tcp_properties_;
 
diff --git a/TAO/tao/Connection_Handler.cpp b/TAO/tao/Connection_Handler.cpp
index 049b1d15a49..66ec38c3c69 100644
--- a/TAO/tao/Connection_Handler.cpp
+++ b/TAO/tao/Connection_Handler.cpp
@@ -17,9 +17,15 @@ ACE_RCSID(tao, Connection_Handler, "$Id$")
 TAO_Connection_Handler::TAO_Connection_Handler (TAO_ORB_Core *orb_core)
   :orb_core_ (orb_core),
    transport_ (0),
-   tss_resources_ (orb_core->get_tss_resources ())
-  //,   is_registered_ (0)
+   tss_resources_ (orb_core->get_tss_resources ()),
+   pending_upcalls_ (1),
+   pending_upcall_lock_ (0)
+
 {
+  // @@todo: We need to have a distinct option/ method in the resource
+  // factory for this and TAO_Transport.
+  this->pending_upcall_lock_ =
+    this->orb_core_->resource_factory ()->create_cached_connection_lock ();
 }
 
 
@@ -30,6 +36,8 @@ TAO_Connection_Handler::~TAO_Connection_Handler (void)
   this->orb_core_ = 0;
   this->tss_resources_ = 0;
   TAO_Transport::release (this->transport_);
+
+  delete this->pending_upcall_lock_;
 }
 
 
diff --git a/TAO/tao/Connection_Handler.h b/TAO/tao/Connection_Handler.h
index ae74b4f8ad1..f18c6c54ed1 100644
--- a/TAO/tao/Connection_Handler.h
+++ b/TAO/tao/Connection_Handler.h
@@ -6,7 +6,7 @@
  *
  *  $Id$
  *
- *  @author Balachandran  Natarajan  <bala@cs.wustl.edu>
+ *  @author Balachandran Natarajan  <bala@cs.wustl.edu>
  */
 //=============================================================================
 
@@ -101,10 +101,17 @@ protected:
   /// Object.
   int svc_i (void);
 
+  /// Increment and decrement the number of upcalls that have gone
+  /// through this handler. Returns the upcall count. The calls are
+  /// thread safe..
+  int incr_pending_upcalls (void);
 
+  int decr_pending_upcalls (void);
 
-private:
+  /// Query the upcall count
+  int pending_upcalls (void) const;
 
+private:
   /// Pointer to the TAO_ORB_Core
   TAO_ORB_Core *orb_core_;
 
@@ -114,8 +121,15 @@ private:
   /// Cached tss resources of the ORB that activated this object.
   TAO_ORB_Core_TSS_Resources *tss_resources_;
 
-  /// Are we registered with the reactor?
-  // CORBA::Boolean is_registered_;
+  /// Count nested upcalls on this
+  /// svc_handler i.e., the connection can close during nested upcalls,
+  /// you should not delete the svc_handler until the stack unwinds
+  /// from the nested upcalls.
+  long pending_upcalls_;
+
+  /// Lock for the <pending_upcalls_>. We can have more than one
+  /// thread trying to access.
+  ACE_Lock *pending_upcall_lock_;
 };
 
 #if defined (__ACE_INLINE__)
diff --git a/TAO/tao/Connection_Handler.inl b/TAO/tao/Connection_Handler.inl
index 1be59aafb4b..49ac55ad5b8 100644
--- a/TAO/tao/Connection_Handler.inl
+++ b/TAO/tao/Connection_Handler.inl
@@ -10,19 +10,6 @@ TAO_Connection_Handler::TAO_Connection_Handler (void)
 {
 }
 
-/*ACE_INLINE CORBA::Boolean
-TAO_Connection_Handler::is_registered (void)
-{
-  return this->is_registered_;
-}
-
-ACE_INLINE void
-TAO_Connection_Handler::is_registered (CORBA::Boolean flag)
-{
-  this->is_registered_ = flag;
-}
-*/
-
 ACE_INLINE TAO_ORB_Core *
 TAO_Connection_Handler::orb_core (void)
 {
@@ -40,3 +27,32 @@ TAO_Connection_Handler::transport (void)
 {
   return this->transport_;
 }
+
+
+ACE_INLINE int
+TAO_Connection_Handler::incr_pending_upcalls (void)
+{
+  ACE_GUARD_RETURN (ACE_Lock,
+                    ace_mon,
+                    *this->pending_upcall_lock_, -1);
+
+  return ++this->pending_upcalls_;
+
+
+}
+
+ACE_INLINE int
+TAO_Connection_Handler::decr_pending_upcalls (void)
+{
+  ACE_GUARD_RETURN (ACE_Lock,
+                    ace_mon,
+                    *this->pending_upcall_lock_, -1);
+
+  return --this->pending_upcalls_;
+}
+
+ACE_INLINE int
+TAO_Connection_Handler::pending_upcalls (void) const
+{
+  return this->pending_upcalls_;
+}
diff --git a/TAO/tao/IIOP_Connection_Handler.cpp b/TAO/tao/IIOP_Connection_Handler.cpp
index 8e685d04878..315aede9bf4 100644
--- a/TAO/tao/IIOP_Connection_Handler.cpp
+++ b/TAO/tao/IIOP_Connection_Handler.cpp
@@ -23,7 +23,6 @@ ACE_RCSID(tao, IIOP_Connection_Handler, "$Id$")
 TAO_IIOP_Connection_Handler::TAO_IIOP_Connection_Handler (ACE_Thread_Manager *t)
   : TAO_IIOP_SVC_HANDLER (t, 0 , 0),
     TAO_Connection_Handler (0),
-    pending_upcalls_ (1),
     tcp_properties_ (0),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
 {
@@ -41,7 +40,6 @@ TAO_IIOP_Connection_Handler::TAO_IIOP_Connection_Handler (TAO_ORB_Core *orb_core
                                                           void *arg)
   : TAO_IIOP_SVC_HANDLER (orb_core->thr_mgr (), 0, 0),
     TAO_Connection_Handler (orb_core),
-    pending_upcalls_ (1),
     tcp_properties_ (ACE_static_cast
                      (TAO_IIOP_Properties *, arg)),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
@@ -206,8 +204,9 @@ TAO_IIOP_Connection_Handler::handle_close (ACE_HANDLE handle,
                handle,
                rm));
 
-  --this->pending_upcalls_;
-  if (this->pending_upcalls_ <= 0)
+  long upcalls = this->decr_pending_upcalls ();
+
+  if (upcalls <= 0)
     {
       if (this->transport ()->wait_strategy ()->is_registered ())
         {
@@ -331,7 +330,7 @@ TAO_IIOP_Connection_Handler::handle_input (ACE_HANDLE)
 
 {
   // Increase the reference count on the upcall that have passed us.
-  this->pending_upcalls_++;
+  this->incr_pending_upcalls ();
 
   this->resume_flag_ = TAO_RESUMES_CONNECTION_HANDLER;
   TAO_Resume_Handle  resume_handle (this->orb_core (),
@@ -340,7 +339,7 @@ TAO_IIOP_Connection_Handler::handle_input (ACE_HANDLE)
   int retval = this->transport ()->handle_input_i (resume_handle);
 
   // The upcall is done. Bump down the reference count
-  if (--this->pending_upcalls_ <= 0)
+  if (this->decr_pending_upcalls () <= 0)
     retval = -1;
 
   if (retval == -1)
diff --git a/TAO/tao/IIOP_Connection_Handler.h b/TAO/tao/IIOP_Connection_Handler.h
index aa8262e039b..1f7d0dead44 100644
--- a/TAO/tao/IIOP_Connection_Handler.h
+++ b/TAO/tao/IIOP_Connection_Handler.h
@@ -138,12 +138,6 @@ protected:
 
 private:
 
-  /// Count nested upcalls on this
-  /// svc_handler i.e., the connection can close during nested upcalls,
-  /// you should not delete the svc_handler until the stack unwinds
-  /// from the nested upcalls.
-  long pending_upcalls_;
-
   /// TCP configuration for this connection.
   TAO_IIOP_Properties *tcp_properties_;
 
diff --git a/TAO/tao/Strategies/DIOP_Connection_Handler.cpp b/TAO/tao/Strategies/DIOP_Connection_Handler.cpp
index ad647e3785d..b35961941bc 100644
--- a/TAO/tao/Strategies/DIOP_Connection_Handler.cpp
+++ b/TAO/tao/Strategies/DIOP_Connection_Handler.cpp
@@ -30,7 +30,6 @@ ACE_RCSID(tao, DIOP_Connect, "$Id$")
 TAO_DIOP_Connection_Handler::TAO_DIOP_Connection_Handler (ACE_Thread_Manager *t)
   : TAO_DIOP_SVC_HANDLER (t, 0 , 0),
     TAO_Connection_Handler (0),
-    pending_upcalls_ (1),
     tcp_properties_ (0),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
 {
@@ -48,7 +47,6 @@ TAO_DIOP_Connection_Handler::TAO_DIOP_Connection_Handler (TAO_ORB_Core *orb_core
                                                           void *arg)
   : TAO_DIOP_SVC_HANDLER (orb_core->thr_mgr (), 0, 0),
     TAO_Connection_Handler (orb_core),
-    pending_upcalls_ (1),
     tcp_properties_ (ACE_static_cast
                      (TAO_DIOP_Properties *, arg)),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
@@ -225,8 +223,10 @@ TAO_DIOP_Connection_Handler::handle_close (ACE_HANDLE handle,
                  handle,
                  rm));
 
-  --this->pending_upcalls_;
-  if (this->pending_upcalls_ <= 0)
+  long pending =
+    this->decr_pending_upcalls ();
+
+  if (pending <= 0)
     {
       // @@ Why are we doing checks for is_registered flags here if the
       // handlers are not registered with the reactor? - Bala
@@ -343,7 +343,7 @@ int
 TAO_DIOP_Connection_Handler::handle_input (ACE_HANDLE)
 {
   // Increase the reference count on the upcall that have passed us.
-  this->pending_upcalls_++;
+  this->incr_pending_upcalls ();
 
   this->resume_flag_ = TAO_RESUMES_CONNECTION_HANDLER;
 
@@ -362,7 +362,7 @@ TAO_DIOP_Connection_Handler::handle_input (ACE_HANDLE)
     }
 
   // The upcall is done. Bump down the reference count
-  if (--this->pending_upcalls_ <= 0)
+  if (this->decr_pending_upcalls () <= 0)
     retval = -1;
 
   if (retval == -1)
diff --git a/TAO/tao/Strategies/DIOP_Connection_Handler.h b/TAO/tao/Strategies/DIOP_Connection_Handler.h
index 6116a33b086..d5583fa1c5b 100644
--- a/TAO/tao/Strategies/DIOP_Connection_Handler.h
+++ b/TAO/tao/Strategies/DIOP_Connection_Handler.h
@@ -173,13 +173,6 @@ protected:
   // DIOP Additions - End
 
 private:
-
-  /// Count nested upcalls on this
-  /// svc_handler i.e., the connection can close during nested upcalls,
-  /// you should not delete the svc_handler until the stack unwinds
-  /// from the nested upcalls.
-  u_long pending_upcalls_;
-
   /// TCP configuration for this connection.
   TAO_DIOP_Properties *tcp_properties_;
 
diff --git a/TAO/tao/Strategies/SHMIOP_Connection_Handler.cpp b/TAO/tao/Strategies/SHMIOP_Connection_Handler.cpp
index e8206171fa4..877b85cd075 100644
--- a/TAO/tao/Strategies/SHMIOP_Connection_Handler.cpp
+++ b/TAO/tao/Strategies/SHMIOP_Connection_Handler.cpp
@@ -25,7 +25,6 @@ ACE_RCSID(Strategies, SHMIOP_Connection_Handler, "$Id$")
 TAO_SHMIOP_Connection_Handler::TAO_SHMIOP_Connection_Handler (ACE_Thread_Manager *t)
   : TAO_SHMIOP_SVC_HANDLER (t, 0 , 0),
     TAO_Connection_Handler (0),
-    pending_upcalls_ (1),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
 {
   // This constructor should *never* get called, it is just here to
@@ -42,7 +41,6 @@ TAO_SHMIOP_Connection_Handler::TAO_SHMIOP_Connection_Handler (TAO_ORB_Core *orb_
                                                               void *)
   : TAO_SHMIOP_SVC_HANDLER (orb_core->thr_mgr (), 0, 0),
     TAO_Connection_Handler (orb_core),
-    pending_upcalls_ (1),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
 {
   TAO_SHMIOP_Transport* specific_transport = 0;
@@ -182,8 +180,10 @@ TAO_SHMIOP_Connection_Handler::handle_close (ACE_HANDLE handle,
                  handle,
                  rm));
 
-  --this->pending_upcalls_;
-  if (this->pending_upcalls_ <= 0)
+  long pending =
+    this->decr_pending_upcalls ();
+
+  if (pending <= 0)
     {
       if (this->transport ()->wait_strategy ()->is_registered ())
         {
@@ -268,7 +268,7 @@ int
 TAO_SHMIOP_Connection_Handler::handle_input (ACE_HANDLE)
 {
   // Increase the reference count on the upcall that have passed us.
-  this->pending_upcalls_++;
+  this->incr_pending_upcalls ();
 
   this->resume_flag_ = TAO_RESUMES_CONNECTION_HANDLER;
 
@@ -278,7 +278,7 @@ TAO_SHMIOP_Connection_Handler::handle_input (ACE_HANDLE)
   int retval = this->transport ()->handle_input_i (resume_handle);
 
   // The upcall is done. Bump down the reference count
-  if (--this->pending_upcalls_ <= 0)
+  if (this->decr_pending_upcalls () <= 0)
     retval = -1;
 
   if (retval == -1)
diff --git a/TAO/tao/Strategies/SHMIOP_Connection_Handler.h b/TAO/tao/Strategies/SHMIOP_Connection_Handler.h
index f6ec45d346b..7b1bc6d569c 100644
--- a/TAO/tao/Strategies/SHMIOP_Connection_Handler.h
+++ b/TAO/tao/Strategies/SHMIOP_Connection_Handler.h
@@ -109,13 +109,6 @@ protected:
   virtual int handle_input (ACE_HANDLE = ACE_INVALID_HANDLE);
 
 private:
-
-  /// Count nested upcalls on this
-  /// svc_handler i.e., the connection can close during nested upcalls,
-  /// you should not delete the svc_handler until the stack unwinds
-  /// from the nested upcalls.
-  long pending_upcalls_;
-
   /// Flag that we will be passing to the event handler to indicate
   /// whether the handle will be resumed by the method or not.
   int resume_flag_;
diff --git a/TAO/tao/Strategies/UIOP_Connection_Handler.cpp b/TAO/tao/Strategies/UIOP_Connection_Handler.cpp
index 14a5f1cb8bc..91d7e0b3a28 100644
--- a/TAO/tao/Strategies/UIOP_Connection_Handler.cpp
+++ b/TAO/tao/Strategies/UIOP_Connection_Handler.cpp
@@ -27,7 +27,6 @@ ACE_RCSID(Strategies, UIOP_Connection_Handler, "$Id$")
 TAO_UIOP_Connection_Handler::TAO_UIOP_Connection_Handler (ACE_Thread_Manager *t)
   : TAO_UIOP_SVC_HANDLER (t, 0 , 0),
     TAO_Connection_Handler (0),
-    pending_upcalls_ (1),
     uiop_properties_ (0),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
 {
@@ -45,7 +44,6 @@ TAO_UIOP_Connection_Handler::TAO_UIOP_Connection_Handler (TAO_ORB_Core *orb_core
                                                           void *arg)
   : TAO_UIOP_SVC_HANDLER (orb_core->thr_mgr (), 0, 0),
     TAO_Connection_Handler (orb_core),
-    pending_upcalls_ (1),
     uiop_properties_ (ACE_static_cast
                      (TAO_UIOP_Properties *, arg)),
     resume_flag_ (TAO_DOESNT_RESUME_CONNECTION_HANDLER)
@@ -166,8 +164,9 @@ TAO_UIOP_Connection_Handler::handle_close (ACE_HANDLE handle,
                  handle,
                  rm));
 
-  --this->pending_upcalls_;
-  if (this->pending_upcalls_ <= 0)
+  long pending = this->decr_pending_upcalls ();
+
+  if (pending <= 0)
     {
       if (this->transport ()->wait_strategy ()->is_registered ())
         {
@@ -245,7 +244,7 @@ TAO_UIOP_Connection_Handler::add_transport_to_cache (void)
 int
 TAO_UIOP_Connection_Handler::handle_input (ACE_HANDLE)
 {
-  this->pending_upcalls_++;
+  this->incr_pending_upcalls ();
 
   this->resume_flag_ = TAO_RESUMES_CONNECTION_HANDLER;
 
@@ -256,7 +255,7 @@ TAO_UIOP_Connection_Handler::handle_input (ACE_HANDLE)
     this->transport ()->handle_input_i (resume_handle);
 
   // The upcall is done. Bump down the reference count
-  if (--this->pending_upcalls_ <= 0)
+  if (this->decr_pending_upcalls () <= 0)
     retval = -1;
 
   if (retval == -1)
diff --git a/TAO/tao/Strategies/UIOP_Connection_Handler.h b/TAO/tao/Strategies/UIOP_Connection_Handler.h
index 41db582d5cb..fb681019fa8 100644
--- a/TAO/tao/Strategies/UIOP_Connection_Handler.h
+++ b/TAO/tao/Strategies/UIOP_Connection_Handler.h
@@ -127,12 +127,6 @@ protected:
 
 private:
 
-  /// Count nested upcalls on this
-  /// svc_handler i.e., the connection can close during nested upcalls,
-  /// you should not delete the svc_handler until the stack unwinds
-  /// from the nested upcalls.
-  long pending_upcalls_;
-
   /// TCP configuration for this connection.
   TAO_UIOP_Properties *uiop_properties_;