ChangeLogTag: Wed Mar 21 15:17:58 2001 Balachandran Natarajan <bala@cs.wustl.edu>

7 files changed, 76 insertions, 49 deletions

diff --git a/TAO/ChangeLogs/ChangeLog-02a b/TAO/ChangeLogs/ChangeLog-02a
index bd1dbc27cbd..f22781ae87f 100644
--- a/TAO/ChangeLogs/ChangeLog-02a
+++ b/TAO/ChangeLogs/ChangeLog-02a
@@ -1,3 +1,17 @@
+Wed Mar 21 15:17:58 2001  Balachandran Natarajan  <bala@cs.wustl.edu>
+
+	* tao/Transport.cpp: 
+	* tao/Transport.h:
+	* tao/Transport.inl:
+	* tao/Transport_Cache_Manager.h:
+	* tao/Transport_Cache_Manager.cpp:
+	* tao/Transport_Cache_Manager.inl: It was observed during testing
+	  that there was a race condition in the code. The race condition
+	  was because of unsynchronized access to the hash_map_entry that
+	  TAO_Transport stores. This is a first attempt to solve the
+	  problem. The problem has been solved by protecting access and
+	  trying to hide the Hash_Map_Entry from direct access.
+
 Wed Mar 21 13:48:44 2001  Balachandran Natarajan  <bala@cs.wustl.edu>
 
 	* docs/Options.html: Added the missing default reactor type to the
diff --git a/TAO/tao/Transport.cpp b/TAO/tao/Transport.cpp
index a0641aadc23..daaeee617ec 100644
--- a/TAO/tao/Transport.cpp
+++ b/TAO/tao/Transport.cpp
@@ -251,7 +251,7 @@ TAO_Transport::leader_follower_condition_variable (void)
 int
 TAO_Transport::tear_listen_point_list (TAO_InputCDR &)
 {
-  ACE_NOTSUP_RETURN(-1);
+  ACE_NOTSUP_RETURN (-1);
 }
 
 int
@@ -265,11 +265,8 @@ TAO_Transport::connection_handler_closing (void)
 {
   this->transition_handler_state ();
 
-  // purge_entry on the Transport_Cache
-  if (this->cache_map_entry_ != 0)
-    {
-      this->orb_core_->transport_cache ().purge_entry (this->cache_map_entry_);
-    }
+  this->orb_core_->transport_cache ().purge_entry (
+    this->cache_map_entry_);
 
   // This should be the last thing we do here
   TAO_Transport::release(this);
@@ -286,15 +283,8 @@ TAO_Transport::_duplicate (TAO_Transport* transport)
 {
   if (transport != 0)
     {
-      transport->increment();
-#if 0
-      ACE_DEBUG ((LM_DEBUG,
-                  "(%P|%t) TAO_Transport::duplicate, "
-                  "%s (%x) releasing %8x "
-                  "TAO_Transport::release count == %d\n",
-                  what, who, transport, count));
-#endif
-  }
+      transport->increment ();
+    }
   return transport;
 }
 
@@ -304,13 +294,7 @@ TAO_Transport::release (TAO_Transport* transport)
   if (transport != 0)
     {
       int count = transport->decrement ();
-#if 0
-      ACE_DEBUG ((LM_DEBUG,
-                  "(%P|%t) TAO_Transport::release, "
-                  "%s (%x) releasing %8x "
-                  "TAO_Transport::release count == %d\n",
-                  what, who, transport, count));
-#endif
+
       if (count == 0)
         {
           delete transport;
@@ -318,8 +302,8 @@ TAO_Transport::release (TAO_Transport* transport)
       else if (count < 0)
         {
           ACE_ERROR ((LM_ERROR,
-                      "(%P|%t) TAO_Transport::release, "
-                      "reference countis less than zero: %d\n",
+                      ACE_TEXT ("(%P|%t) TAO_Transport::release, ")
+                      ACE_TEXT ("reference countis less than zero: %d\n"),
                       count));
           ACE_OS::abort ();
         }
@@ -330,10 +314,7 @@ int
 TAO_Transport::recache_transport (TAO_Transport_Descriptor_Interface *desc)
 {
   // First purge our entry
-  if (this->cache_map_entry_ != 0)
-    {
-      this->orb_core_->transport_cache ().purge_entry (this->cache_map_entry_);
-    }
+  this->orb_core_->transport_cache ().purge_entry (this->cache_map_entry_);
 
   // Then add ourselves to the cache
   return this->orb_core_->transport_cache ().cache_transport (desc,
@@ -343,20 +324,18 @@ TAO_Transport::recache_transport (TAO_Transport_Descriptor_Interface *desc)
 void
 TAO_Transport::mark_invalid (void)
 {
-  if (this->cache_map_entry_ != 0)
-    {
-      this->cache_map_entry_->int_id_.recycle_state (ACE_RECYCLABLE_PURGABLE_BUT_NOT_IDLE);
-    }
+  // @@ Do we need this method at all??
+  this->orb_core_->transport_cache ().mark_invalid (
+    this->cache_map_entry_);
+
+
+
 }
 
 int
 TAO_Transport::make_idle (void)
 {
-  if (this->cache_map_entry_ != 0)
-    {
-      return this->orb_core_->transport_cache ().make_idle (this->cache_map_entry_);
-    }
-  return 0;
+  return this->orb_core_->transport_cache ().make_idle (this->cache_map_entry_);
 }
 
 void
@@ -371,9 +350,5 @@ TAO_Transport::close_connection (void)
     }
 
   // Purge the entry
-  if (this->cache_map_entry_ != 0)
-    {
-      this->orb_core_->transport_cache ().purge_entry (this->cache_map_entry_);
-      this->cache_map_entry_ = 0;
-    }
+  this->orb_core_->transport_cache ().purge_entry (this->cache_map_entry_);
 }
diff --git a/TAO/tao/Transport.h b/TAO/tao/Transport.h
index a945f52cdc8..7bfb51e626d 100644
--- a/TAO/tao/Transport.h
+++ b/TAO/tao/Transport.h
@@ -504,8 +504,10 @@ public:
   int recache_transport (TAO_Transport_Descriptor_Interface* desc);
 
   /// Set/Get the Cache Map entry
-  void cache_map_entry (TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *entry);
-  TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *cache_map_entry (void);
+  void cache_map_entry (
+    TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *entry);
+
+  //  TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *& cache_map_entry (void);
 
   void mark_invalid (void);
 
@@ -541,7 +543,8 @@ protected:
   /// Global orbcore resource.
   TAO_ORB_Core *orb_core_;
 
-  // Our entry in the cache
+  /// Our entry in the cache. We dont own this. It is here for our
+  /// convinience. We cannot just change things around.
   TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *cache_map_entry_;
 
   /// Strategy to decide whether multiple requests can be sent over the
diff --git a/TAO/tao/Transport.inl b/TAO/tao/Transport.inl
index 47aeb1ee783..53059e65190 100644
--- a/TAO/tao/Transport.inl
+++ b/TAO/tao/Transport.inl
@@ -100,12 +100,13 @@ TAO_Transport::dequeue_all (void)
     }
 }
 
-ACE_INLINE TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *
+/*ACE_INLINE TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *
 TAO_Transport::cache_map_entry (void)
 {
   return this->cache_map_entry_;
 }
-   
+*/
+
 ACE_INLINE void
 TAO_Transport::cache_map_entry (
     TAO_Transport_Cache_Manager::HASH_MAP_ENTRY *entry)
diff --git a/TAO/tao/Transport_Cache_Manager.cpp b/TAO/tao/Transport_Cache_Manager.cpp
index 8ff53219404..9e2e289082c 100644
--- a/TAO/tao/Transport_Cache_Manager.cpp
+++ b/TAO/tao/Transport_Cache_Manager.cpp
@@ -268,8 +268,13 @@ TAO_Transport_Cache_Manager::close_i (ACE_Handle_Set & /*handle_set*/)
 int
 TAO_Transport_Cache_Manager::purge_entry_i (HASH_MAP_ENTRY *&entry)
 {
-  // Remove the enrty from the Map
-  int retval =  this->cache_map_.unbind (entry);
+  int retval = 0;
+
+  if (entry)
+    {
+      // Remove the enrty from the Map
+     retval =  this->cache_map_.unbind (entry);
+    }
 
   // Set the entry pointer to zero
   entry = 0;
@@ -277,6 +282,18 @@ TAO_Transport_Cache_Manager::purge_entry_i (HASH_MAP_ENTRY *&entry)
   return retval;
 }
 
+void
+TAO_Transport_Cache_Manager::mark_invalid_i (HASH_MAP_ENTRY *&entry)
+{
+  if (entry)
+    {
+      // Mark the entry as not usable
+      entry->int_id_.recycle_state (ACE_RECYCLABLE_PURGABLE_BUT_NOT_IDLE);
+    }
+
+}
+
+
 
 int
 TAO_Transport_Cache_Manager::
diff --git a/TAO/tao/Transport_Cache_Manager.h b/TAO/tao/Transport_Cache_Manager.h
index 32c43497f6f..f56a3e348fc 100644
--- a/TAO/tao/Transport_Cache_Manager.h
+++ b/TAO/tao/Transport_Cache_Manager.h
@@ -131,6 +131,9 @@ public:
   /// Purge the entry from the Cache Map
   int purge_entry (HASH_MAP_ENTRY *&);
 
+  /// Mark the entry as invalid for use but keep it in cache.
+  void mark_invalid (HASH_MAP_ENTRY *&);
+
   /// Make the entry idle and ready for use.
   int make_idle (HASH_MAP_ENTRY *&entry);
 
@@ -190,6 +193,9 @@ private:
   /// Purge the entry from the Cache Map
   int purge_entry_i (HASH_MAP_ENTRY *&entry);
 
+  /// Mark the entry as invalid for use but keep it in cache.
+  void mark_invalid_i (HASH_MAP_ENTRY *&);
+
 private:
 
   /**
diff --git a/TAO/tao/Transport_Cache_Manager.inl b/TAO/tao/Transport_Cache_Manager.inl
index beeef7e5000..0b5932ee694 100644
--- a/TAO/tao/Transport_Cache_Manager.inl
+++ b/TAO/tao/Transport_Cache_Manager.inl
@@ -110,6 +110,17 @@ TAO_Transport_Cache_Manager::purge_entry (HASH_MAP_ENTRY *&entry)
   return this->purge_entry_i (entry);
 }
 
+ACE_INLINE void
+TAO_Transport_Cache_Manager::mark_invalid (HASH_MAP_ENTRY *&entry)
+{
+  ACE_MT (ACE_GUARD (ACE_Lock,
+                     guard,
+                     *this->cache_lock_));
+
+  this->mark_invalid_i (entry);
+}
+
+
 
 ACE_INLINE int
 TAO_Transport_Cache_Manager::make_idle (HASH_MAP_ENTRY *&entry)