ChangeLogTag:Sat Apr 29 19:41:15 2000 Carlos O'Ryan <coryan@uci.edu>

8 files changed, 104 insertions, 73 deletions

diff --git a/TAO/ChangeLogs/ChangeLog-02a b/TAO/ChangeLogs/ChangeLog-02a
index 81ef971ca8f..7104d17d131 100644
--- a/TAO/ChangeLogs/ChangeLog-02a
+++ b/TAO/ChangeLogs/ChangeLog-02a
@@ -1,3 +1,22 @@
+Sat Apr 29 19:41:15 2000  Carlos O'Ryan  <coryan@uci.edu>
+
+	* tao/Invocation.cpp:
+	* tao/Transport_Mux_Strategy.h:
+	* tao/Transport_Mux_Strategy.cpp:
+	  If a timeout expires while sending a request we need to clear
+	  the reply dispatcher from the transport object.  There was no
+	  way to do that.
+
+	* tests/Timeout/client.cpp:
+	* tests/Timeout/server.cpp:
+	* tests/Timeout/test_i.cpp:
+	  Change the test to ensure that the ORB does not crash if a reply
+	  is received after the request timeout has expired.
+	  Reduce debugging output.
+
+	* tests/Timeout/run_test.pl:
+	  Extend the range of timeouts that the we check for.
+
 Sat Apr 29 19:41:39 2000  Darrell Brunsch <brunsch@uci.edu>
 
         * orbsvcs/orbsvcs/RTEvent_Static.dsp:
diff --git a/TAO/tao/Invocation.cpp b/TAO/tao/Invocation.cpp
index f0faa8caa31..a6c8df35c6b 100644
--- a/TAO/tao/Invocation.cpp
+++ b/TAO/tao/Invocation.cpp
@@ -784,9 +784,12 @@ TAO_GIOP_Twoway_Invocation::invoke_i (CORBA::Environment &ACE_TRY_ENV)
   // preallocated reply dispatcher.
 
   // Bind.
+
+  TAO_Transport_Mux_Strategy *tms = this->transport_->tms ();
+
   int retval =
-    this->transport_->tms ()->bind_dispatcher (this->op_details_.request_id (),
-                                               &this->rd_);
+    tms->bind_dispatcher (this->op_details_.request_id (),
+                          &this->rd_);
   if (retval == -1)
     {
       // @@ What is the right way to handle this error?
@@ -858,6 +861,7 @@ TAO_GIOP_Twoway_Invocation::invoke_i (CORBA::Environment &ACE_TRY_ENV)
 
   if (reply_error == -1)
     {
+      tms->unbind_dispatcher (this->op_details_.request_id ());
       if (errno == ETIME)
         {
           // Just a timeout, don't close the connection or
@@ -1048,9 +1052,14 @@ TAO_GIOP_Oneway_Invocation::invoke (CORBA::Environment &ACE_TRY_ENV)
   // The rest of this function is very similar to
   // TWO_GIOP_Twoway_Invocation::invoke_i, because we must
   // wait for a reply. See comments in that code.
+  // @@ Jeff: is it possible to factor out this code into a common
+  // case class or something?  In fact, what is the difference between
+  // the two classes now?
+  TAO_Transport_Mux_Strategy *tms = this->transport_->tms ();
+
   int retval =
-    this->transport_->tms ()->bind_dispatcher (this->op_details_.request_id (),
-                                               &rd);
+    tms->bind_dispatcher (this->op_details_.request_id (),
+                          &rd);
   if (retval == -1)
     {
       // @@ What is the right way to handle this error?
@@ -1098,6 +1107,7 @@ TAO_GIOP_Oneway_Invocation::invoke (CORBA::Environment &ACE_TRY_ENV)
   // Check the reply error.
   if (reply_error == -1)
     {
+      tms->unbind_dispatcher (this->op_details_.request_id ());
       if (errno == ETIME)
         {
           // Just a timeout, don't close the connection or
@@ -1271,6 +1281,8 @@ TAO_GIOP_Locate_Request_Invocation::invoke (CORBA::Environment &ACE_TRY_ENV)
   // preallocated reply dispatcher.
 
   // Bind.
+  TAO_Transport_Mux_Strategy *tms = this->transport_->tms ();
+
   int retval =
     this->transport_->tms ()->bind_dispatcher (this->op_details_.request_id (),
                                                &this->rd_);
@@ -1330,6 +1342,7 @@ TAO_GIOP_Locate_Request_Invocation::invoke (CORBA::Environment &ACE_TRY_ENV)
   // Check the reply error.
   if (reply_error == -1)
     {
+      tms->unbind_dispatcher (this->op_details_.request_id ());
       if (errno == ETIME)
         {
           // Just a timeout, don't close the connection or
diff --git a/TAO/tao/Transport_Mux_Strategy.cpp b/TAO/tao/Transport_Mux_Strategy.cpp
index 3972ed3060b..5d7d5e8b0ce 100644
--- a/TAO/tao/Transport_Mux_Strategy.cpp
+++ b/TAO/tao/Transport_Mux_Strategy.cpp
@@ -24,6 +24,11 @@ TAO_Transport_Mux_Strategy::bind_dispatcher (CORBA::ULong,
   return rd->leader_follower_condition_variable (this->transport_);
 }
 
+void
+TAO_Transport_Mux_Strategy::unbind_dispatcher (CORBA::ULong)
+{
+}
+
 // *********************************************************************
 
 TAO_Exclusive_TMS::TAO_Exclusive_TMS (TAO_Transport *transport)
@@ -72,6 +77,15 @@ TAO_Exclusive_TMS::bind_dispatcher (CORBA::ULong request_id,
                                                       rd);
 }
 
+void
+TAO_Exclusive_TMS::unbind_dispatcher (CORBA::ULong request_id)
+{
+  if (this->request_id_ != request_id)
+    return;
+  this->request_id_ = 0xdeadbeef; // @@ What is a good value???
+  this->rd_ = 0;
+}
+
 int
 TAO_Exclusive_TMS::dispatch_reply (CORBA::ULong request_id,
                                    CORBA::ULong reply_status,
@@ -214,6 +228,13 @@ TAO_Muxed_TMS::bind_dispatcher (CORBA::ULong request_id,
                                                       rd);
 }
 
+void
+TAO_Muxed_TMS::unbind_dispatcher (CORBA::ULong request_id)
+{
+  TAO_Reply_Dispatcher *rd = 0;
+  (void) this->dispatcher_table_.unbind (request_id, rd);
+}
+
 int
 TAO_Muxed_TMS::dispatch_reply (CORBA::ULong request_id,
                                CORBA::ULong reply_status,
diff --git a/TAO/tao/Transport_Mux_Strategy.h b/TAO/tao/Transport_Mux_Strategy.h
index c3a9b1ac35b..b06b2baedad 100644
--- a/TAO/tao/Transport_Mux_Strategy.h
+++ b/TAO/tao/Transport_Mux_Strategy.h
@@ -66,6 +66,13 @@ public:
   // Bind the dispatcher with the request id. Commonalities in the
   // derived class implementations is kept here.
 
+  virtual void unbind_dispatcher (CORBA::ULong request_id);
+  // Unbind the dispatcher, the client is no longer waiting for the
+  // request, for example, because the request timedout.
+  // The strategy can (must) cleanup any resources associated with the
+  // request.
+  // A later reply for that request should be ignored.
+
   virtual int dispatch_reply (CORBA::ULong request_id,
                               CORBA::ULong reply_status,
                               const TAO_GIOP_Version& version,
@@ -120,40 +127,21 @@ public:
   virtual ~TAO_Exclusive_TMS (void);
   // Destructor.
 
+  // = Please read the documentation in the TAO_Transport_Mux_Strategy
+  //   class.
   virtual CORBA::ULong request_id (void);
-  // Generate and return an unique request id for the current
-  // invocation.
-
   virtual int bind_dispatcher (CORBA::ULong request_id,
                                TAO_Reply_Dispatcher *rh);
-  // Bind the dispatcher with the request id.
-
+  virtual void unbind_dispatcher (CORBA::ULong request_id);
   virtual int dispatch_reply (CORBA::ULong request_id,
                               CORBA::ULong reply_status,
                               const TAO_GIOP_Version& version,
                               IOP::ServiceContextList& reply_ctx,
                               TAO_GIOP_Message_State* message_state);
-  // Dispatch the reply for <request_id>, cleanup any resources
-  // allocated for that request.
-
   virtual TAO_GIOP_Message_State *get_message_state (void);
-  // Return the pre-allocated message state.
-
   virtual void destroy_message_state (TAO_GIOP_Message_State *);
-  // No op in this strategy.
-
   virtual int idle_after_send (void);
-  // Request has been just sent, but the reply is not received. Idle
-  // the transport now.
-
   virtual int idle_after_reply (void);
-  // Request is sent and the reply is received. Idle the transport
-  // now.
-
-  // virtual int reply_received (const CORBA::ULong request_id);
-  // Check whether the reply has been receieved for the request with
-  // <request_id>. Return 0 if no, 1 on yes and -1 if the request_id
-  // is invalid or there are errors.
 
 protected:
   CORBA::ULong request_id_generator_;
@@ -197,36 +185,20 @@ public:
   // Generate and return an unique request id for the current
   // invocation.
 
+  // = Please read the documentation in the TAO_Transport_Mux_Strategy
+  //   class.
   virtual int bind_dispatcher (CORBA::ULong request_id,
                                TAO_Reply_Dispatcher *rh);
-  // Bind the dispatcher with the request id.
-
+  virtual void unbind_dispatcher (CORBA::ULong request_id);
   virtual int dispatch_reply (CORBA::ULong request_id,
                               CORBA::ULong reply_status,
                               const TAO_GIOP_Version& version,
                               IOP::ServiceContextList& reply_ctx,
                               TAO_GIOP_Message_State* message_state);
-  // Dispatch the reply for <request_id>, cleanup any resources
-  // allocated for that request.
-
   virtual TAO_GIOP_Message_State *get_message_state (void);
-  // Return the message state.
-
   virtual void destroy_message_state (TAO_GIOP_Message_State *);
-  // No op in this strategy.
-
   virtual int idle_after_send (void);
-  // Request has been just sent, but the reply is not received. Idle
-  // the transport now.
-
   virtual int idle_after_reply (void);
-  // Request is sent and the reply is received. Idle the transport
-  // now.
-
-  //  virtual int reply_received (const CORBA::ULong request_id);
-  // Check whether the reply has been receieved for the request with
-  // <request_id>. Return 0 if no, 1 on yes and -1 if the request_id
-  // is invalid or there are errors.
 
 protected:
   CORBA::ULong request_id_generator_;
diff --git a/TAO/tests/Timeout/client.cpp b/TAO/tests/Timeout/client.cpp
index e6dcabc62ea..786bfbf88f9 100644
--- a/TAO/tests/Timeout/client.cpp
+++ b/TAO/tests/Timeout/client.cpp
@@ -46,7 +46,8 @@ parse_args (int argc, char *argv[])
 }
 
 void
-send_echo (Simple_Server_ptr server,
+send_echo (CORBA::ORB_ptr orb,
+           Simple_Server_ptr server,
            CORBA::Long t,
            CORBA::Environment &ACE_TRY_ENV)
 {
@@ -59,13 +60,13 @@ send_echo (Simple_Server_ptr server,
   ACE_CATCH (CORBA::TIMEOUT, timeout)
     {
       // Trap this exception and continue...
-      ACE_DEBUG ((LM_DEBUG,
-                  "==> Trapped a TIMEOUT exception (expected)\n"));
+      // ACE_DEBUG ((LM_DEBUG,
+      //             "==> Trapped a TIMEOUT exception (expected)\n"));
 
       // Sleep so the server can send the reply...
       ACE_Time_Value tv (max_timeout / 1000,
                          (max_timeout % 1000) * 1000);
-      ACE_OS::sleep (tv);
+      orb->run (tv);
     }
   ACE_ENDTRY;
 }
@@ -146,14 +147,17 @@ int main (int argc, char* argv[])
       policy_list[0]->destroy (ACE_TRY_ENV);
       policy_list[0] = CORBA::Policy::_nil ();
 
+      ACE_DEBUG ((LM_DEBUG,
+                  "client (%P) testing from %d to %d milliseconds\n",
+                  min_timeout, max_timeout));
       for (CORBA::Long t = min_timeout; t != max_timeout; ++t)
         {
-          ACE_DEBUG ((LM_DEBUG,
-                      "\n================================\n"
-                      "Trying with timeout = %d\n", t));
+          //ACE_DEBUG ((LM_DEBUG,
+          //            "\n================================\n"
+          //            "Trying with timeout = %d\n", t));
 
-          ACE_DEBUG ((LM_DEBUG,
-                      "Cleanup ORB/Thread/Object policies\n"));
+          // ACE_DEBUG ((LM_DEBUG,
+          //            "Cleanup ORB/Thread/Object policies\n"));
 
           policy_list.length (0);
           policy_manager->set_policy_overrides (policy_list,
@@ -165,11 +169,11 @@ int main (int argc, char* argv[])
                                                 ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
-          send_echo (server.in (), t, ACE_TRY_ENV);
+          send_echo (orb.in (), server.in (), t, ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
-          ACE_DEBUG ((LM_DEBUG,
-                      "client(%P) Set the ORB policies\n"));
+          // ACE_DEBUG ((LM_DEBUG,
+          //             "client(%P) Set the ORB policies\n"));
 
           policy_list.length (1);
           policy_list[0] =
@@ -183,14 +187,14 @@ int main (int argc, char* argv[])
                                                 ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
-          send_echo (server.in (), t, ACE_TRY_ENV);
+          send_echo (orb.in (), server.in (), t, ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
           policy_list[0]->destroy (ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
-          ACE_DEBUG ((LM_DEBUG,
-                      "client(%P) Set the thread policies\n"));
+          // ACE_DEBUG ((LM_DEBUG,
+          //             "client(%P) Set the thread policies\n"));
 
           policy_list.length (1);
           policy_list[0] =
@@ -204,20 +208,21 @@ int main (int argc, char* argv[])
                                                 ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
-          send_echo (server.in (), t, ACE_TRY_ENV);
+          send_echo (orb.in (), server.in (), t, ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
           policy_list[0]->destroy (ACE_TRY_ENV);
           ACE_TRY_CHECK;
 
-          ACE_DEBUG ((LM_DEBUG,
-                      "client(%P) Use the object policies\n"));
-          send_echo (timeout_server.in (), t, ACE_TRY_ENV);
+          // ACE_DEBUG ((LM_DEBUG,
+          //            "client(%P) Use the object policies\n"));
+          send_echo (orb.in (), timeout_server.in (), t, ACE_TRY_ENV);
           ACE_TRY_CHECK;
         }
 
-      ACE_DEBUG ((LM_DEBUG,
-                  "\n\n\nclient(%P) Test completed, resynch with server\n"));
+      // ACE_DEBUG ((LM_DEBUG,
+      //             "\n\n\nclient(%P) Test completed, "
+      //             "resynch with server\n"));
       policy_list.length (0);
       policy_manager->set_policy_overrides (policy_list,
                                             CORBA::SET_OVERRIDE,
@@ -228,7 +233,7 @@ int main (int argc, char* argv[])
                                             ACE_TRY_ENV);
       ACE_TRY_CHECK;
 
-      send_echo (server.in (), 0, ACE_TRY_ENV);
+      send_echo (orb.in (), server.in (), 0, ACE_TRY_ENV);
       ACE_TRY_CHECK;
 
       server->shutdown (ACE_TRY_ENV);
diff --git a/TAO/tests/Timeout/run_test.pl b/TAO/tests/Timeout/run_test.pl
index 02a96bb7954..017d45036b1 100755
--- a/TAO/tests/Timeout/run_test.pl
+++ b/TAO/tests/Timeout/run_test.pl
@@ -29,7 +29,7 @@ if (ACE::waitforfile_timed ($iorfile, 5) == -1) {
 $CL = Process::Create ($EXEPREFIX."client$EXE_EXT ",
                        " -ORBDebugLevel 0"
                        ." -k file://$iorfile "
-                       . " -l 6 -h 14");
+                       . " -l 2 -h 28");
 
 $client = $CL->TimedWait (60);
 if ($client == -1) {
diff --git a/TAO/tests/Timeout/server.cpp b/TAO/tests/Timeout/server.cpp
index d7430caf857..6ee05cbf91f 100644
--- a/TAO/tests/Timeout/server.cpp
+++ b/TAO/tests/Timeout/server.cpp
@@ -70,7 +70,7 @@ main (int argc, char *argv[])
 	orb->object_to_string (server.in (), ACE_TRY_ENV);
       ACE_TRY_CHECK;
 
-      ACE_DEBUG ((LM_DEBUG, "Activated as <%s>\n", ior.in ()));
+      // ACE_DEBUG ((LM_DEBUG, "Activated as <%s>\n", ior.in ()));
 
       // If the ior_output_file exists, output the ior to it
       if (ior_output_file != 0)
@@ -90,7 +90,7 @@ main (int argc, char *argv[])
 
       orb->run ();
 
-      ACE_DEBUG ((LM_DEBUG, "event loop finished\n"));
+      // ACE_DEBUG ((LM_DEBUG, "event loop finished\n"));
     }
   ACE_CATCHANY
     {
diff --git a/TAO/tests/Timeout/test_i.cpp b/TAO/tests/Timeout/test_i.cpp
index 1383acf1de8..f51c48e0ad7 100644
--- a/TAO/tests/Timeout/test_i.cpp
+++ b/TAO/tests/Timeout/test_i.cpp
@@ -22,8 +22,8 @@ Simple_Server_i::echo (CORBA::Long x,
 {
   ACE_Time_Value tv (msecs / 1000, (msecs % 1000) * 1000);
 
-  ACE_DEBUG ((LM_DEBUG, "server (%P) Sleeping for %d msecs\n",
-              tv.msec ()));
+  // ACE_DEBUG ((LM_DEBUG, "server (%P) Sleeping for %d msecs\n",
+  //             tv.msec ()));
   ACE_OS::sleep (tv);
 
   return x;
@@ -33,6 +33,7 @@ void
 Simple_Server_i::shutdown (CORBA::Environment& ACE_TRY_ENV)
   ACE_THROW_SPEC ((CORBA::SystemException))
 {
-  ACE_DEBUG ((LM_DEBUG, "Received shutdown request from client\n"));
+  ACE_DEBUG ((LM_DEBUG,
+              "server (%P) Received shutdown request from client\n"));
   this->orb_->shutdown (0, ACE_TRY_ENV);
 }