Wed Apr 5 22:39:28 UTC 2006 Adam Mitz <mitza@ociweb.com>

10 files changed, 466 insertions, 6 deletions

diff --git a/TAO/ChangeLog b/TAO/ChangeLog
index 12d1de35db3..f736ef2bc84 100644
--- a/TAO/ChangeLog
+++ b/TAO/ChangeLog
@@ -1,3 +1,22 @@
+Wed Apr  5 22:39:28 UTC 2006  Adam Mitz  <mitza@ociweb.com>
+
+        * tests/Bug_2494_Regression/Bug_2494_Regression.mpc:
+        * tests/Bug_2494_Regression/README:
+        * tests/Bug_2494_Regression/client.cpp:
+        * tests/Bug_2494_Regression/run_test.pl:
+        * tests/Bug_2494_Regression/server.cpp:
+        * tests/Bug_2494_Regression/test.idl:
+        * tests/Bug_2494_Regression/test_i.h:
+        * tests/Bug_2494_Regression/test_i.inl:
+        * tests/Bug_2494_Regression/test_i.cpp:
+
+          Added a regression test for Bugzilla bug #2494.
+        
+        * tao/Transport.cpp (~Transport):
+        
+          Added back the ACE_ASSERTS, needed for the regression test above.
+          See Bugzilla bug #2494.
+
 Tue Apr  4 22:03:35 UTC 2006  Wallace Zhang  <zhangw@ociweb.com>
 
         * orbsvcs/examples/ImR/Advanced/TestClient.h:
@@ -86,11 +105,18 @@ Fri Mar 31 17:19:13 UTC 2006  Adam Mitz  <mitza@ociweb.com>
 
         * tao/Transport.cpp:
 
-          Fixes a problem where the server would terminate
-          when the client process is killed.  This was due to one thread
-          closing a transport while another was still using it.
-          With this change, the transport will check if the handler is
-          still registered in the reactor before scheduling output.
+          See bugzilla bug #2494 for full details.  This fixes a race condition
+          where one thread sends data out of the transport (drain_queue_helper)
+          but then gets scheduled out before reaching the flushing strategy.
+          Meanwhile another thread runs on the same transport and notices that
+          the client has closed the connection.  This thread closes the
+          transport.  The first thread then continues and attempts to register
+          with the reactor to do further output (schedule_output_i), however
+          the connection handler has already been removed from the reactor.
+          This causes problems later on including a memory leak, since a block
+          has been allocated on the tranpsort's queue (in send_reply_message_i)
+          and it will never be deallocated (the transport destructor will run
+          first).
 
 Fri Mar 31 15:17:51 UTC 2006  Jeff Parsons <j.parsons@vanderbilt.edu>
 
diff --git a/TAO/tao/Transport.cpp b/TAO/tao/Transport.cpp
index 5ca39789405..db773dea256 100644
--- a/TAO/tao/Transport.cpp
+++ b/TAO/tao/Transport.cpp
@@ -182,6 +182,11 @@ TAO_Transport::~TAO_Transport (void)
   // By the time the destructor is reached here all the connection stuff
   // *must* have been cleaned up.
 
+  // The following assert is needed for the test "Bug_2494_Regression".
+  // See the bugzilla bug #2494 for details.
+  ACE_ASSERT (this->head_ == 0);
+  ACE_ASSERT (this->cache_map_entry_ == 0);
+
   /*
    * Hook to add code that cleans up components
    * belong to the concrete protocol implementation.
@@ -646,7 +651,7 @@ TAO_Transport::send_reply_message_i (const ACE_Message_Block *mb,
       msg->remove_from_list (this->head_, this->tail_);
       msg->destroy ();
     }
-  
+
   return 1;
 }
 
diff --git a/TAO/tests/Bug_2494_Regression/Bug_2494_Regression.mpc b/TAO/tests/Bug_2494_Regression/Bug_2494_Regression.mpc
new file mode 100644
index 00000000000..ef312844a93
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/Bug_2494_Regression.mpc
@@ -0,0 +1,19 @@
+// -*- MPC -*-
+// $Id$
+
+project(*Server): taoexe, portableserver {
+  idlflags += -Sc -St
+  Source_Files {
+    test_i.cpp
+    server.cpp
+  }
+}
+
+project(*Client): taoexe {
+  idlflags += -Sc -St
+  after += *Server
+  Source_Files {
+    testC.cpp
+    client.cpp
+  }
+}
diff --git a/TAO/tests/Bug_2494_Regression/README b/TAO/tests/Bug_2494_Regression/README
new file mode 100644
index 00000000000..726419360b1
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/README
@@ -0,0 +1,34 @@
+# $Id$
+
+Description:
+
+        This test a modification of MT_Server.  In this case the client is also
+multi-threaded.  The client sends very large strings to the server, which it 
+echoes back, for the sake of keeping the server busy doing I/O.  When the
+client is abruptly killed, the server should close the connection and continue
+running.  
+
+Expected output:
+        The server prints out the IOR of the object it serves and the 
+results of server shutdown (aborted or shutdown cleanly).
+
+        Regression:
+Activated as 
+<IOR:012a2a2a1600000049444c3a53696d706c655f5365727665723a312e30002a2a0100000000000000740000000101022a130000006f6369313332392e6f63697765622e636f6d002a19842a2a1b00000014010f0052535420c93244520b04000000000001000000010000002a020000000000000008000000012a2a2a004f41540100000018000000012a2a2a0100010001000000010001050901010000000000>
+***Client has been killed***
+ACE_ASSERT (32395|3067820976): file Transport.cpp, line 172 assertion 
+failed for 'this->head_ == 0'.Aborting...
+ERROR: cannot find file 
+</tao_builds/mitza/1.4a/ACE_wrappers/TAO/tests/Bug_2494_Regression/server_terminated> 
+-- server has not shut down cleanly.
+(perl script returns 1)
+
+        Correct run:
+Activated as 
+<IOR:012a2a2a1600000049444c3a53696d706c655f5365727665723a312e30002a2a0100000000000000740000000101022a130000006f6369313332392e6f63697765622e636f6d002a34842a2a1b00000014010f0052535488d03244438d04000000000001000000010000002a020000000000000008000000012a2a2a004f41540100000018000000012a2a2a0100010001000000010001050901010000000000>
+***Client has been killed***
+event loop finished
+(perl script returns 0)
+
+How to run:
+        Use the run_test.pl script to run it.
diff --git a/TAO/tests/Bug_2494_Regression/client.cpp b/TAO/tests/Bug_2494_Regression/client.cpp
new file mode 100644
index 00000000000..731a0a3ca01
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/client.cpp
@@ -0,0 +1,137 @@
+// $Id$
+
+#include "ace/Get_Opt.h"
+#include "ace/Task.h"
+#include "ace/OS_NS_string.h"
+#include "testC.h"
+
+ACE_RCSID(Bug_2494_Regression, client, "$Id$")
+
+const char *ior = "file://test.ior";
+int nthreads = 5;
+int do_shutdown = 0;
+
+int
+parse_args (int argc, char *argv[])
+{
+  ACE_Get_Opt get_opts (argc, argv, "xk:n:");
+  int c;
+
+  while ((c = get_opts ()) != -1)
+    switch (c)
+      {
+      case 'x':
+        do_shutdown = 1;
+        break;
+
+      case 'k':
+        ior = get_opts.opt_arg ();
+        break;
+
+      case 'n':
+        nthreads = ACE_OS::atoi (get_opts.opt_arg ());
+        break;
+
+      case '?':
+      default:
+        ACE_ERROR_RETURN ((LM_ERROR,
+                           "usage: %s "
+                           "-k <ior> "
+                           "[-n <nthreads> | -x]"
+                           "\n",
+                           argv [0]),
+                          -1);
+      }
+  // Indicates sucessful parsing of the command line
+  return 0;
+}
+
+namespace
+{
+  const char *twohundredbytes = 
+  "12345678901234567890123456789012345678901234567890"
+  "12345678901234567890123456789012345678901234567890"
+  "12345678901234567890123456789012345678901234567890"
+  "12345678901234567890123456789012345678901234567890"
+  ;
+}
+
+struct Worker : ACE_Task_Base
+{
+
+  Worker (Simple_Server_ptr srv)
+    : srv_(Simple_Server::_duplicate(srv))
+  {
+  }
+
+  Simple_Server_var srv_;
+
+  int svc ()
+  {
+    char* str = CORBA::string_alloc (200*2000 + 1);
+    if (!str) return 1;
+    str[0] = CORBA::Char('\0');
+    for (int i=0; i < 2000; ++i)
+      {
+        ACE_OS::strcat(str, twohundredbytes);
+      }
+
+    while (1)
+      {
+        try
+          {
+            const char *ret = srv_->test_method (str);
+            ACE_TRY_CHECK;
+            if (0 != ACE_OS::strcmp (str, ret)) return 1;
+          }
+        catch (CORBA::Exception& ex)
+          {
+            ACE_PRINT_EXCEPTION (ex, "Exception caught:");
+            return 1;
+          }
+      }
+    return 0;
+  }
+
+};
+
+int
+main (int argc, char *argv[])
+{
+  try
+    {
+      CORBA::ORB_var orb = CORBA::ORB_init (argc, argv);
+
+      if (parse_args (argc, argv) != 0)
+        return 1;
+
+      CORBA::Object_var object = orb->string_to_object (ior);
+
+      Simple_Server_var server = Simple_Server::_narrow (object.in ());
+
+      if (CORBA::is_nil (server.in ()))
+        {
+          ACE_ERROR_RETURN ((LM_ERROR,
+                             "Object reference <%s> is nil\n",
+                             ior), 1);
+        }
+
+      if (do_shutdown)
+        {
+          server->shutdown ();
+        }
+      else
+        {
+          Worker wrk (server.in ());
+          wrk.activate (THR_NEW_LWP|THR_JOINABLE|THR_INHERIT_SCHED, nthreads);
+          wrk.thr_mgr ()->wait ();
+        }
+    }
+  catch (CORBA::Exception& ex)
+    {
+      ACE_PRINT_EXCEPTION (ex, "Exception caught:");
+      return 1;
+    }
+
+  return 0;
+}
diff --git a/TAO/tests/Bug_2494_Regression/server.cpp b/TAO/tests/Bug_2494_Regression/server.cpp
new file mode 100644
index 00000000000..8a87e86c160
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/server.cpp
@@ -0,0 +1,157 @@
+// $Id$
+
+#include "test_i.h"
+#include "ace/Get_Opt.h"
+#include "ace/Task.h"
+
+ACE_RCSID(Bug_2494_Regression, server, "$Id$")
+
+const char *ior_output_file = "file://test.ior";
+int nthreads = 4;
+
+int
+parse_args (int argc, char *argv[])
+{
+  ACE_Get_Opt get_opts (argc, argv, "o:n:");
+  int c;
+
+  while ((c = get_opts ()) != -1)
+    switch (c)
+      {
+      case 'o':
+        ior_output_file = get_opts.opt_arg ();
+        break;
+
+      case 'n':
+        nthreads = ACE_OS::atoi (get_opts.opt_arg ());
+        break;
+
+      case '?':
+      default:
+        ACE_ERROR_RETURN ((LM_ERROR,
+                           "usage:  %s "
+                           "-o <iorfile>"
+                           "\n",
+                           argv [0]),
+                          -1);
+      }
+  // Indicates sucessful parsing of the command line
+  return 0;
+}
+
+class Worker : public ACE_Task_Base
+{
+  // = TITLE
+  //   Run a server thread
+  //
+  // = DESCRIPTION
+  //   Use the ACE_Task_Base class to run server threads
+  //
+public:
+  Worker (CORBA::ORB_ptr orb);
+  // ctor
+
+  virtual int svc (void);
+  // The thread entry point.
+
+private:
+  CORBA::ORB_var orb_;
+  // The orb
+};
+
+int
+main (int argc, char *argv[])
+{
+  try
+    {
+      CORBA::ORB_var orb = CORBA::ORB_init (argc, argv);
+
+      CORBA::Object_var poa_object =
+        orb->resolve_initial_references("RootPOA");
+
+      if (CORBA::is_nil (poa_object.in ()))
+        ACE_ERROR_RETURN ((LM_ERROR,
+                           " (%P|%t) Unable to initialize the POA.\n"),
+                          1);
+
+      PortableServer::POA_var root_poa =
+        PortableServer::POA::_narrow (poa_object.in ());
+
+      PortableServer::POAManager_var poa_manager = root_poa->the_POAManager ();
+
+      if (parse_args (argc, argv) != 0)
+        return 1;
+
+      Simple_Server_i server_impl (orb.in ());
+
+      Simple_Server_var server = server_impl._this ();
+
+      CORBA::String_var ior = orb->object_to_string (server.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)
+        {
+          FILE *output_file= ACE_OS::fopen (ior_output_file, "w");
+          if (output_file == 0)
+            ACE_ERROR_RETURN ((LM_ERROR,
+                               "Cannot open output file for writing IOR: %s",
+                               ior_output_file),
+                              1);
+          ACE_OS::fprintf (output_file, "%s", ior.in ());
+          ACE_OS::fclose (output_file);
+        }
+
+      poa_manager->activate ();
+
+      Worker worker (orb.in ());
+      if (worker.activate (THR_NEW_LWP | THR_JOINABLE,
+                           nthreads) != 0)
+        ACE_ERROR_RETURN ((LM_ERROR,
+                           "Cannot activate client threads\n"),
+                          1);
+
+      worker.thr_mgr ()->wait ();
+
+      ACE_DEBUG ((LM_DEBUG, "event loop finished\n"));
+
+      const char *fname = "server_terminated";
+      FILE *output_file= ACE_OS::fopen (fname, "w");
+      if (output_file == 0)
+        ACE_ERROR_RETURN ((LM_ERROR,
+                           "Cannot open output file for writing: ",
+                           fname),
+                          1);
+      ACE_OS::fprintf (output_file, "%s", "OK\n");
+      ACE_OS::fclose (output_file);
+    }
+  catch (CORBA::Exception& ex)
+    {
+      ACE_PRINT_EXCEPTION (ACE_ANY_EXCEPTION,
+                           "Exception caught:");
+      return 1;
+    }
+
+  return 0;
+}
+
+// ****************************************************************
+
+Worker::Worker (CORBA::ORB_ptr orb)
+  :  orb_ (CORBA::ORB::_duplicate (orb))
+{
+}
+
+int
+Worker::svc (void)
+{
+  try
+    {
+      this->orb_->run ();
+    }
+  catch (CORBA::Exception&)
+    {
+    }
+  return 0;
+}
diff --git a/TAO/tests/Bug_2494_Regression/test.idl b/TAO/tests/Bug_2494_Regression/test.idl
new file mode 100644
index 00000000000..335009e627f
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/test.idl
@@ -0,0 +1,10 @@
+//
+// $Id$
+//
+
+interface Simple_Server
+{
+  string test_method (in string str);
+
+  oneway void shutdown ();
+};
diff --git a/TAO/tests/Bug_2494_Regression/test_i.cpp b/TAO/tests/Bug_2494_Regression/test_i.cpp
new file mode 100644
index 00000000000..bfccb378f26
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/test_i.cpp
@@ -0,0 +1,29 @@
+// $Id$
+
+#include "test_i.h"
+#include "tao/debug.h"
+#include "ace/OS_NS_unistd.h"
+
+#if !defined(__ACE_INLINE__)
+#include "test_i.inl"
+#endif /* __ACE_INLINE__ */
+
+ACE_RCSID(Bug_2494_Regression, test_i, "$Id$")
+
+char *
+Simple_Server_i::test_method (const char *x)
+    ACE_THROW_SPEC ((CORBA::SystemException))
+{
+  if (TAO_debug_level > 0)
+    ACE_DEBUG ((LM_DEBUG, "Request in thread %t\n"));
+  ACE_Time_Value tv (2);
+  ACE_OS::sleep (tv);
+  return CORBA::string_dup(x);
+}
+
+void
+Simple_Server_i::shutdown ()
+    ACE_THROW_SPEC ((CORBA::SystemException))
+{
+  this->orb_->shutdown (0);
+}
diff --git a/TAO/tests/Bug_2494_Regression/test_i.h b/TAO/tests/Bug_2494_Regression/test_i.h
new file mode 100644
index 00000000000..cd8971dc7a6
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/test_i.h
@@ -0,0 +1,36 @@
+// $Id$
+
+#ifndef TAO_BUG_2494_REGRESSION_TEST_I_H
+#define TAO_BUG_2494_REGRESSION_TEST_I_H
+
+#include "testS.h"
+
+class Simple_Server_i : public POA_Simple_Server
+{
+  // = TITLE
+  //   Simpler Server implementation
+  //
+  // = DESCRIPTION
+  //   Implements the Simple_Server interface in test.idl
+  //
+public:
+  Simple_Server_i (CORBA::ORB_ptr orb);
+  // ctor
+
+  // = The Simple_Server methods.
+  char *test_method (const char *x)
+    ACE_THROW_SPEC ((CORBA::SystemException));
+
+  void shutdown ()
+    ACE_THROW_SPEC ((CORBA::SystemException));
+
+private:
+  CORBA::ORB_var orb_;
+  // The ORB
+};
+
+#if defined(__ACE_INLINE__)
+#include "test_i.inl"
+#endif /* __ACE_INLINE__ */
+
+#endif /* TAO_BUG_2494_REGRESSION_TEST_I_H */
diff --git a/TAO/tests/Bug_2494_Regression/test_i.inl b/TAO/tests/Bug_2494_Regression/test_i.inl
new file mode 100644
index 00000000000..97524552ff4
--- /dev/null
+++ b/TAO/tests/Bug_2494_Regression/test_i.inl
@@ -0,0 +1,7 @@
+// $Id$
+
+ACE_INLINE
+Simple_Server_i::Simple_Server_i (CORBA::ORB_ptr orb)
+  :  orb_ (CORBA::ORB::_duplicate (orb))
+{
+}