ChangeLogTag: Wed Sep 5 12:35:33 2001 Balachandran Natarajan <bala@cs.wustl.edu>

10 files changed, 132 insertions, 35 deletions

diff --git a/TAO/ChangeLogs/ChangeLog-02a b/TAO/ChangeLogs/ChangeLog-02a
index f18399e3ef8..4b465ad27f3 100644
--- a/TAO/ChangeLogs/ChangeLog-02a
+++ b/TAO/ChangeLogs/ChangeLog-02a
@@ -1,3 +1,53 @@
+Wed Sep  5 12:35:33 2001  Balachandran Natarajan  <bala@cs.wustl.edu>
+
+	* tao/Endpoint.h: Added a lock that is used to lock when a thread
+	  does remote object address lookups. Theoretically speaking we
+	  shouldnt be putting the lock in the TAO_Endpoint class. But as
+	  the lock is required for most of the protocols supported in TAO
+	  we have pushed it here. We also believe that other protocols may
+	  need a lock during lookups.
+
+	* tao/IIOP_Enpoint.i: Hold a lock in object_addr (). There was a
+	  subtle race condition in that method. In object_addr (void) the
+	  code first checks for object_addr_.get_type() != AF_INET and, if
+	  so, it calls ACE_INET_Addr::set(). ACE_INET_Addr::set() sets
+	  type type field to type field to AF_INET, zeros the inet_addr
+	  structure and then does a hostname lookup. 
+
+	  If two threads enter the object_addr() method the first may end
+	  up blocked for a while in the hostname lookup.  The second
+	  thread will see that AF_INET is set and return an (zero'ed)
+	  ACE_INET_Addr structure. 
+	  
+	  The race can happen from TAO_IIOP_Connector::connect() when
+	  multiple threads attempt to talk to the same object.  You will
+	  sometimes see transient exceptions as one of the threads tries
+	  to connect to the (invalid) inet_addr. This could potentially
+	  fix #189, but it hasnt been tested out properly. Thanks to
+	  <pphillip@opentext.com> for nailining the probelm and suggesting
+	  this fix. This should fix #1017.
+
+	* tao/Strategies/DIOP_Endpoint.i:
+	* tao/Strategies/SHMIOP_Endpoint.i: Applied the same fix.
+
+	* orbsvcs/orbsvcs/SSLIOP/SSLIOP_Endpoint.i: Just added a comment
+	  telling that we shouldnt be holding the lock as the lock is being
+	  held in the IIOP class.
+		
+	* tao/IIOP_Transport.cpp:
+	* tao/Strategies/DIOP_Transport.cpp:
+	* tao/Strategies/SHMIOP_Transport.cpp:
+	* tao/Strategies/UIOP_Transport.cpp:
+	* orbsvcs/orbsvcs/SSLIOP/SSLIOP_Transport.cpp: Do not print out
+	  error messages if the recv () returns from a timeout. This extra
+	  print statements in a thread-per-connection case, when a thread
+	  timedout, was causing more confusion than it tried addressing.
+
+Wed Sep  5 11:39:33 2001  Balachandran Natarajan  <bala@cs.wustl.edu>
+
+	* ta/IIOP_
+	* tao/Strategies/DIOP_Endpoint.i:
+
 Wed  Sep  5 7:20:17 2001  Balachandran Natarajan  <bala@cs.wustl.edu>
 
 	* orbsvcs/tests/AVStreams/Multicast/ftp.dsp:
diff --git a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Endpoint.i b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Endpoint.i
index 44fad01abf2..ff35c8e41c8 100644
--- a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Endpoint.i
+++ b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Endpoint.i
@@ -30,6 +30,8 @@ TAO_SSLIOP_Endpoint::object_addr (void) const
     ACE_const_cast (ACE_INET_Addr &,
                     this->object_addr_);
 
+  // Dont hold the address lookup lock here. If you do then you will
+  // have a deadlock  as we hold the lock in IIOP_Endpoint.
   ssl_addr = this->iiop_endpoint_->object_addr ();
   ssl_addr.set_port_number (this->ssl_component_.port);
 
diff --git a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Transport.cpp b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Transport.cpp
index 5dfab65a3ab..239bda35e7c 100644
--- a/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Transport.cpp
+++ b/TAO/orbsvcs/orbsvcs/SSLIOP/SSLIOP_Transport.cpp
@@ -118,7 +118,9 @@ TAO_SSLIOP_Transport::recv_i (char *buf,
 
   // Most of the errors handling is common for
   // Now the message has been read
-  if (n == -1 && TAO_debug_level > 4)
+  if (n == -1 &&
+      TAO_debug_level > 4 &&
+      errno != ETIME)
     {
       ACE_DEBUG ((LM_DEBUG,
                   ACE_TEXT ("TAO (%P|%t) - %p \n"),
diff --git a/TAO/tao/Endpoint.h b/TAO/tao/Endpoint.h
index e5602a279ca..82c4f13092c 100644
--- a/TAO/tao/Endpoint.h
+++ b/TAO/tao/Endpoint.h
@@ -86,6 +86,16 @@ public:
 
   /// Return a hash value for this object.
   virtual CORBA::ULong hash (void) = 0;
+
+protected:
+
+  /// Lock for the address lookup.
+  /// @@todo: This lock should be strategised so that we dont lock in
+  /// single threaded configurations. I am not able to do this now as
+  /// most of the information is available in the ORB_Core which is
+  /// not available here...
+  TAO_SYNCH_MUTEX addr_lookup_lock_;
+
 private:
 
   // Endpoints should not be copied.
@@ -102,6 +112,8 @@ private:
    * currently used for RTCORBA only.
    */
   CORBA::Short priority_;
+
+
 };
 
 #if defined (__ACE_INLINE__)
diff --git a/TAO/tao/IIOP_Endpoint.i b/TAO/tao/IIOP_Endpoint.i
index ac5c2b8415b..8e478b7da04 100644
--- a/TAO/tao/IIOP_Endpoint.i
+++ b/TAO/tao/IIOP_Endpoint.i
@@ -16,20 +16,27 @@ TAO_IIOP_Endpoint::object_addr (void) const
     ACE_const_cast (TAO_IIOP_Endpoint *,
                     this);
 
-  if (this->object_addr_.get_type () != AF_INET
-      && endpoint->object_addr_.set (this->port_,
-                                     this->host_.in ()) == -1)
-    {
-      // If this call fails, it most likely due a hostname lookup
-      // failure caused by a DNS misconfiguration.  If a request is
-      // made to the object at the given host and port, then a
-      // CORBA::TRANSIENT() exception should be thrown.
-
-      // Invalidate the ACE_INET_Addr.  This is used as a flag to
-      // denote that ACE_INET_Addr initialization failed.
-      endpoint->object_addr_.set_type (-1);
-    }
-
+  // Begin a dummy scope for the lock
+  {
+    ACE_GUARD_RETURN (TAO_SYNCH_MUTEX,
+                      guard,
+                      endpoint->addr_lookup_lock_,
+                      this->object_addr_ );
+
+    if (this->object_addr_.get_type () != AF_INET
+        && endpoint->object_addr_.set (this->port_,
+                                       this->host_.in ()) == -1)
+      {
+        // If this call fails, it most likely due a hostname lookup
+        // failure caused by a DNS misconfiguration.  If a request is
+        // made to the object at the given host and port, then a
+        // CORBA::TRANSIENT() exception should be thrown.
+
+        // Invalidate the ACE_INET_Addr.  This is used as a flag to
+        // denote that ACE_INET_Addr initialization failed.
+        endpoint->object_addr_.set_type (-1);
+      }
+  }
   return this->object_addr_;
 }
 
diff --git a/TAO/tao/IIOP_Transport.cpp b/TAO/tao/IIOP_Transport.cpp
index 85e5f4b1d84..46d339d8c9e 100644
--- a/TAO/tao/IIOP_Transport.cpp
+++ b/TAO/tao/IIOP_Transport.cpp
@@ -85,9 +85,11 @@ TAO_IIOP_Transport::recv_i (char *buf,
                                                        len,
                                                        max_wait_time);
 
-  // Most of the errors handling is common for
-  // Now the message has been read
-  if (n == -1 && TAO_debug_level > 4)
+  // Do not print the error message if it is a timeout, which could
+  // occur in thread-per-connection.
+  if (n == -1 &&
+      TAO_debug_level > 4 &&
+      errno != ETIME)
     {
       ACE_DEBUG ((LM_DEBUG,
                   ACE_TEXT ("TAO (%P|%t) - %p \n"),
@@ -101,8 +103,13 @@ TAO_IIOP_Transport::recv_i (char *buf,
       if (errno == EWOULDBLOCK)
         return 0;
 
+
       return -1;
     }
+
+  // Most of the errors handling is common for
+  // Now the message has been read
+
   // @@ What are the other error handling here??
   else if (n == 0)
     {
diff --git a/TAO/tao/Strategies/DIOP_Endpoint.i b/TAO/tao/Strategies/DIOP_Endpoint.i
index ad9b9d97a30..b3489cbc445 100644
--- a/TAO/tao/Strategies/DIOP_Endpoint.i
+++ b/TAO/tao/Strategies/DIOP_Endpoint.i
@@ -16,20 +16,27 @@ TAO_DIOP_Endpoint::object_addr (void) const
     ACE_const_cast (TAO_DIOP_Endpoint *,
                     this);
 
-  if (this->object_addr_.get_type () != AF_INET
-      && endpoint->object_addr_.set (this->port_,
-                                    this->host_.in ()) == -1)
-    {
-      // If this call fails, it most likely due a hostname lookup
-      // failure caused by a DNS misconfiguration.  If a request is
-      // made to the object at the given host and port, then a
-      // CORBA::TRANSIENT() exception should be thrown.
-
-      // Invalidate the ACE_INET_Addr.  This is used as a flag to
-      // denote that ACE_INET_Addr initialization failed.
-      endpoint->object_addr_.set_type (-1);
-    }
-
+  // Begin artificial scoping here..
+  {
+    ACE_GUARD_RETURN (TAO_SYNCH_MUTEX,
+                      guard,
+                      endpoint->addr_lookup_lock_,
+                      this->object_addr_ );
+
+    if (this->object_addr_.get_type () != AF_INET
+        && endpoint->object_addr_.set (this->port_,
+                                       this->host_.in ()) == -1)
+      {
+        // If this call fails, it most likely due a hostname lookup
+        // failure caused by a DNS misconfiguration.  If a request is
+        // made to the object at the given host and port, then a
+        // CORBA::TRANSIENT() exception should be thrown.
+
+        // Invalidate the ACE_INET_Addr.  This is used as a flag to
+        // denote that ACE_INET_Addr initialization failed.
+        endpoint->object_addr_.set_type (-1);
+      }
+  }
   return this->object_addr_;
 }
 
diff --git a/TAO/tao/Strategies/SHMIOP_Endpoint.i b/TAO/tao/Strategies/SHMIOP_Endpoint.i
index b09a0ea3fb5..9319b0cdfc0 100644
--- a/TAO/tao/Strategies/SHMIOP_Endpoint.i
+++ b/TAO/tao/Strategies/SHMIOP_Endpoint.i
@@ -16,7 +16,14 @@ TAO_SHMIOP_Endpoint::object_addr (void) const
     ACE_const_cast (TAO_SHMIOP_Endpoint *,
                     this);
 
-  if (this->object_addr_.get_type () != AF_INET
+  // Begin a dummy scope for the lock
+  {
+    ACE_GUARD_RETURN (TAO_SYNCH_MUTEX,
+                      guard,
+                      endpoint->addr_lookup_lock_,
+                      this->object_addr_ );
+
+    if (this->object_addr_.get_type () != AF_INET
       && endpoint->object_addr_.set (this->port_,
                                     this->host_.in ()) == -1)
     {
@@ -29,6 +36,7 @@ TAO_SHMIOP_Endpoint::object_addr (void) const
       // denote that ACE_INET_Addr initialization failed.
       endpoint->object_addr_.set_type (-1);
     }
+  }
 
   return this->object_addr_;
 }
diff --git a/TAO/tao/Strategies/SHMIOP_Transport.cpp b/TAO/tao/Strategies/SHMIOP_Transport.cpp
index 6cd79c5620f..64fbe74061f 100644
--- a/TAO/tao/Strategies/SHMIOP_Transport.cpp
+++ b/TAO/tao/Strategies/SHMIOP_Transport.cpp
@@ -115,7 +115,7 @@ TAO_SHMIOP_Transport::recv_i (char *buf,
 
   if (n == 0 || n == -1)
     {
-      if (TAO_debug_level > 3)
+      if (TAO_debug_level > 3 && errno != ETIME)
         {
           ACE_DEBUG ((LM_DEBUG,
                       ACE_TEXT ("TAO (%P|%t) - %p \n"),
diff --git a/TAO/tao/Strategies/UIOP_Transport.cpp b/TAO/tao/Strategies/UIOP_Transport.cpp
index 2baf994c1ac..f31de7af50f 100644
--- a/TAO/tao/Strategies/UIOP_Transport.cpp
+++ b/TAO/tao/Strategies/UIOP_Transport.cpp
@@ -88,7 +88,9 @@ TAO_UIOP_Transport::recv_i (char *buf,
 
   // Most of the errors handling is common for
   // Now the message has been read
-  if (n == -1 && TAO_debug_level > 4)
+  if (n == -1 &&
+      TAO_debug_level > 4 &&
+      errno != ETIME)
     {
       ACE_DEBUG ((LM_DEBUG,
                   ACE_TEXT ("TAO (%P|%t) - %p \n"),