ChangeLogTag: Mon Nov 22 18:02:19 1999 Irfan Pyarali <irfan@cs.wustl.edu>

3 files changed, 68 insertions, 19 deletions

diff --git a/ChangeLog-99b b/ChangeLog-99b
index d38b8147e66..cf7728dc0d0 100644
--- a/ChangeLog-99b
+++ b/ChangeLog-99b
@@ -1,7 +1,36 @@
+Mon Nov 22 18:02:19 1999  Irfan Pyarali  <irfan@cs.wustl.edu>
+
+	* ace/Strategies_T.cpp:
+	* ace/Cached_Connect_Strategy_T.cpp:
+
+	  There was a strange interaction in
+	  find_or_create_svc_handler_i() because of these two changes:
+
+	  Fri Nov 12 00:30:41 1999  Irfan Pyarali  <irfan@cs.wustl.edu>
+
+	  and
+
+	  Mon Jul 19 22:46:54 1999  Irfan Pyarali  <irfan@cs.wustl.edu>
+
+	  Basically, <sh> was assigned to the newly created svc_handler.
+	  It had, however, not been added to the connection map nor had it
+	  been assigned a hint or recycler, i.e., it was partially
+	  initialized.
+
+	  However, we let go of the connector lock during the OS level
+	  connect call, hence exposing the partially initialized
+	  svc_handler to other threads.
+
+	  The solution is to use a temporary variable until the
+	  svc_handler is completely initialized.  After this we can assign
+	  <sh> to <potential_handler>. Note that making a new svc_handler,
+	  connecting remotely, binding to the map, and assigning of the
+	  hint and recycler should be atomic to the outside world.
+
 Mon Nov 22 17:06:34 1999  Darrell Brunsch <brunsch@cs.wustl.edu>
 
         * bin/auto_compile_win32.pl:  Changed the project file for
-          ImplRepo_Service.  Thanks to Derek Dominish 
+          ImplRepo_Service.  Thanks to Derek Dominish
           <Derek.Dominish@Australia.Boeing.com> for pointing this out.
 
 Mon Nov 22 14:15:23 1999  Douglas C. Schmidt  <schmidt@danzon.cs.wustl.edu>
@@ -73,7 +102,7 @@ Fri Nov 19 02:21:11 1999  Darrell Brunsch <brunsch@cs.wustl.edu>
         * ace/Configuration.h:
         * ace/Configuration.cpp:
           Altered the Unicode macros used so it compiles in the two
-          Unicode configurations.  Thanks to Ruibiao Qiu 
+          Unicode configurations.  Thanks to Ruibiao Qiu
           <ruibiao@arl.wustl.edu> for noticing this.
 
 Thu Nov 18 22:49:09 1999  David L. Levine  <levine@cs.wustl.edu>
diff --git a/ace/Cached_Connect_Strategy_T.cpp b/ace/Cached_Connect_Strategy_T.cpp
index 8e0bb7ada5c..749737b93f9 100644
--- a/ace/Cached_Connect_Strategy_T.cpp
+++ b/ace/Cached_Connect_Strategy_T.cpp
@@ -175,12 +175,21 @@ ACE_Cached_Connect_Strategy_Ex<ACE_T2>::find_or_create_svc_handler_i
       // Set the flag
       found = 0;
 
+      // We need to use a temporary variable here since we are not
+      // allowed to change <sh> because other threads may use this
+      // when we let go of the lock during the OS level connect.
+      //
+      // Note that making a new svc_handler, connecting remotely,
+      // binding to the map, and assigning of the hint and recycler
+      // should be atomic to the outside world.
+      SVC_HANDLER *potential_handler = 0;
+
       // Create a new svc_handler
-      if (this->make_svc_handler (sh) == -1)
+      if (this->make_svc_handler (potential_handler) == -1)
         return -1;
 
       // Connect using the svc_handler.
-      if (this->cached_connect (sh,
+      if (this->cached_connect (potential_handler,
                                 remote_addr,
                                 timeout,
                                 local_addr,
@@ -188,9 +197,8 @@ ACE_Cached_Connect_Strategy_Ex<ACE_T2>::find_or_create_svc_handler_i
                                 flags,
                                 perms) == -1)
         {
-          // Close the svc handler and reset <sh>.
-          sh->close (0);
-          sh = 0;
+          // Close the svc handler.
+          potential_handler->close (0);
 
           return -1;
         }
@@ -198,16 +206,18 @@ ACE_Cached_Connect_Strategy_Ex<ACE_T2>::find_or_create_svc_handler_i
         {
           // Insert the new SVC_HANDLER instance into the cache.
           if (this->connection_cache_.bind (search_addr,
-                                            sh,
+                                            potential_handler,
                                             entry) == -1)
             {
               // Close the svc handler and reset <sh>.
-              sh->close (0);
-              sh = 0;
+              potential_handler->close (0);
 
               return -1;
             }
 
+          // Everything succeeded as planned. Assign <sh> to <potential_handler>.
+          sh = potential_handler;
+
           // Set the recycler and the recycling act
           this->assign_recycler (sh, this, entry);
         }
diff --git a/ace/Strategies_T.cpp b/ace/Strategies_T.cpp
index d60ee48a2de..85c7241e4e6 100644
--- a/ace/Strategies_T.cpp
+++ b/ace/Strategies_T.cpp
@@ -620,13 +620,22 @@ ACE_Cached_Connect_Strategy<SVC_HANDLER, ACE_PEER_CONNECTOR_2, MUTEX>::find_or_c
       // Set the flag
       found = 0;
 
+      // We need to use a temporary variable here since we are not
+      // allowed to change <sh> because other threads may use this
+      // when we let go of the lock during the OS level connect.
+      //
+      // Note that making a new svc_handler, connecting remotely,
+      // binding to the map, and assigning of the hint and recycler
+      // should be atomic to the outside world.
+      SVC_HANDLER *potential_handler = 0;
+
       // Create a new svc_handler
-      if (this->make_svc_handler (sh) == -1)
+      if (this->make_svc_handler (potential_handler) == -1)
         return -1;
 
       // Actively establish the connection.  This is a timed blocking
       // connect.
-      if (this->new_connection (sh,
+      if (this->new_connection (potential_handler,
                                 remote_addr,
                                 timeout,
                                 local_addr,
@@ -642,9 +651,8 @@ ACE_Cached_Connect_Strategy<SVC_HANDLER, ACE_PEER_CONNECTOR_2, MUTEX>::find_or_c
           if (errno == EWOULDBLOCK)
             errno = ENOTSUP;
 
-          // Close the svc handler and reset <sh>.
-          sh->close (0);
-          sh = 0;
+          // Close the svc handler.
+          potential_handler->close (0);
 
           return -1;
         }
@@ -652,16 +660,18 @@ ACE_Cached_Connect_Strategy<SVC_HANDLER, ACE_PEER_CONNECTOR_2, MUTEX>::find_or_c
         {
           // Insert the new SVC_HANDLER instance into the cache.
           if (this->connection_map_.bind (search_addr,
-                                          sh,
+                                          potential_handler,
                                           entry) == -1)
             {
-              // Close the svc handler and reset <sh>.
-              sh->close (0);
-              sh = 0;
+              // Close the svc handler.
+              potential_handler->close (0);
 
               return -1;
             }
 
+          // Everything succeeded as planned. Assign <sh> to <potential_handler>.
+          sh = potential_handler;
+
           // Set the recycler and the recycling act
           this->assign_recycler (sh, this, entry);
         }