Merge pull request #921 from jwillemsen/jwi-imr

When the ImR sendc_ping fails with a timeout mark the server as timedout instead of dead

6 files changed, 97 insertions, 48 deletions

diff --git a/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.cpp b/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.cpp
index 248c73a882a..0934f68f194 100644
--- a/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.cpp
+++ b/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.cpp
@@ -21,8 +21,7 @@ AsyncListManager::AsyncListManager (const Locator_Repository *repo,
    first_ (0),
    how_many_ (0),
    waiters_ (0),
-   refcount_ (1),
-   lock_ ()
+   refcount_ (1)
 {
 }
 
@@ -45,7 +44,7 @@ AsyncListManager::poa (void)
 void
 AsyncListManager::init_list (void)
 {
-  CORBA::ULong len =
+  CORBA::ULong const len =
     static_cast<CORBA::ULong> (this->repo_->servers ().current_size ());
   Locator_Repository::SIMap::ENTRY* entry = 0;
   Locator_Repository::SIMap::CONST_ITERATOR it (this->repo_->servers ());
@@ -78,7 +77,7 @@ AsyncListManager::init_list (void)
             {
               if (!evaluate_status (i, l->status(), info->pid))
                 {
-                  this->waiters_++;
+                  ++this->waiters_;
                 }
               else
                 {
@@ -316,7 +315,6 @@ AsyncListManager::ping_replied (CORBA::ULong index, LiveStatus status, int pid)
 AsyncListManager *
 AsyncListManager::_add_ref (void)
 {
-  ACE_GUARD_RETURN (TAO_SYNCH_MUTEX, mon, this->lock_, 0);
   ++this->refcount_;
   return this;
 }
@@ -324,11 +322,8 @@ AsyncListManager::_add_ref (void)
 void
 AsyncListManager::_remove_ref (void)
 {
-  int count = 0;
-  {
-    ACE_GUARD (TAO_SYNCH_MUTEX, mon, this->lock_);
-    count = --this->refcount_;
-  }
+  int const count = --this->refcount_;
+
   if (count == 0)
     {
       delete this;
@@ -360,7 +355,7 @@ ListLiveListener::~ListLiveListener (void)
 bool
 ListLiveListener::start (void)
 {
-  bool rtn = this->pinger_.add_poll_listener (this);
+  bool const rtn = this->pinger_.add_poll_listener (this);
   this->started_ = true;
   return rtn;
 }
@@ -388,7 +383,9 @@ ListLiveListener::status_changed (LiveStatus status)
   else
     {
       if (this->started_)
-        this->owner_->ping_replied (this->index_, status, this->pid_);
+        {
+          this->owner_->ping_replied (this->index_, status, this->pid_);
+        }
     }
   return true;
 }
diff --git a/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.h b/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.h
index bd722dd71ca..101a0ceb880 100644
--- a/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.h
+++ b/TAO/orbsvcs/ImplRepo_Service/AsyncListManager.h
@@ -19,6 +19,11 @@
 #endif /* ACE_LACKS_PRAGMA_ONCE */
 
 #include "tao/Intrusive_Ref_Count_Handle_T.h"
+#if defined (ACE_HAS_CPP11)
+# include <atomic>
+#else
+# include "ace/Atomic_Op.h"
+#endif /* ACE_HAS_CPP11 */
 #include "LiveCheck.h"
 
 class Locator_Repository;
@@ -73,8 +78,11 @@ class AsyncListManager
   CORBA::ULong first_;
   CORBA::ULong how_many_;
   CORBA::ULong waiters_;
-  int refcount_;
-  TAO_SYNCH_MUTEX lock_;
+#if defined (ACE_HAS_CPP11)
+  std::atomic<int> refcount_;
+#else
+  ACE_Atomic_Op<TAO_SYNCH_MUTEX, int> refcount_;
+#endif /* ACE_HAS_CPP11 */
 };
 
 typedef TAO_Intrusive_Ref_Count_Handle<AsyncListManager> AsyncListManager_ptr;
diff --git a/TAO/orbsvcs/ImplRepo_Service/LiveCheck.cpp b/TAO/orbsvcs/ImplRepo_Service/LiveCheck.cpp
index 8e498bcd55b..84620c5635e 100644
--- a/TAO/orbsvcs/ImplRepo_Service/LiveCheck.cpp
+++ b/TAO/orbsvcs/ImplRepo_Service/LiveCheck.cpp
@@ -12,8 +12,7 @@
 
 LiveListener::LiveListener (const char *server)
   : server_ (server),
-    refcount_ (1),
-    lock_ ()
+    refcount_ (1)
 {
 }
 
@@ -30,13 +29,12 @@ LiveListener::server (void) const
 LiveListener *
 LiveListener::_add_ref (void)
 {
-  ACE_GUARD_RETURN (TAO_SYNCH_MUTEX, mon, this->lock_, 0);
-  ++this->refcount_;
+  int const refcount = ++this->refcount_;
   if (ImR_Locator_i::debug () > 5)
     {
       ORBSVCS_DEBUG ((LM_DEBUG,
                       ACE_TEXT ("(%P|%t) LiveListener::add_ref <%C> count <%d>\n"),
-                      server_.c_str(), refcount_));
+                      server_.c_str(), refcount));
     }
   return this;
 }
@@ -44,17 +42,13 @@ LiveListener::_add_ref (void)
 void
 LiveListener::_remove_ref (void)
 {
-  int count = 0;
-  {
-    ACE_GUARD (TAO_SYNCH_MUTEX, mon, this->lock_);
-    count = --this->refcount_;
-    if (ImR_Locator_i::debug () > 5)
-      {
-        ORBSVCS_DEBUG ((LM_DEBUG,
-                        ACE_TEXT  ("(%P|%t) LiveListener::remove_ref <%C> count <%d>\n"),
-                        server_.c_str(), count));
-      }
-  }
+  int const count = --this->refcount_;
+  if (ImR_Locator_i::debug () > 5)
+    {
+      ORBSVCS_DEBUG ((LM_DEBUG,
+                      ACE_TEXT  ("(%P|%t) LiveListener::remove_ref <%C> count <%d>\n"),
+                      server_.c_str(), count));
+    }
   if (count == 0)
     {
       delete this;
@@ -338,6 +332,12 @@ LiveEntry::pid (void) const
 }
 
 bool
+LiveEntry::may_ping (void) const
+{
+  return this->may_ping_;
+}
+
+bool
 LiveEntry::has_pid (int pid) const
 {
   return this->pid_ == 0 || pid == 0 || pid == this->pid_;
@@ -350,9 +350,9 @@ LiveEntry::validate_ping (bool &want_reping, ACE_Time_Value& next)
     {
       ORBSVCS_DEBUG ((LM_DEBUG,
                       ACE_TEXT ("(%P|%t) LiveEntry::validate_ping, status ")
-                      ACE_TEXT ("<%C> listeners <%d> server <%C> pid <%d> want_reping <%d>\n"),
+                      ACE_TEXT ("<%C> listeners <%d> server <%C> pid <%d> want_reping <%d> may_ping <%d>\n"),
                       status_name (this->liveliness_), this->listeners_.size (),
-                      this->server_.c_str(), this->pid_, want_reping));
+                      this->server_.c_str(), this->pid_, want_reping, this->may_ping_));
     }
 
   if (this->liveliness_ == LS_PING_AWAY ||
@@ -474,15 +474,28 @@ LiveEntry::do_ping (PortableServer::POA_ptr poa)
                           this->server_.c_str()));
         }
     }
+  catch (const CORBA::TIMEOUT &ex)
+    {
+      if (ImR_Locator_i::debug () > 3)
+        {
+          ORBSVCS_DEBUG ((LM_DEBUG,
+                          ACE_TEXT ("(%P|%t) LiveEntry::do_ping, ")
+                          ACE_TEXT ("sendc_ping for server <%C> threw <%C> marking as timed out\n"),
+                          this->server_.c_str(), ex._info ().c_str ()));
+        }
+      this->release_callback ();
+      this->status (LS_TIMEDOUT);
+    }
   catch (const CORBA::Exception &ex)
     {
       if (ImR_Locator_i::debug () > 3)
         {
           ORBSVCS_DEBUG ((LM_DEBUG,
                           ACE_TEXT ("(%P|%t) LiveEntry::do_ping, ")
-                          ACE_TEXT ("sendc_ping for server <%C> threw <%C>\n"),
+                          ACE_TEXT ("sendc_ping for server <%C> threw <%C> marking as dead\n"),
                           this->server_.c_str(), ex._info ().c_str ()));
         }
+      this->release_callback ();
       this->status (LS_DEAD);
     }
 }
@@ -805,8 +818,8 @@ LiveCheck::handle_timeout (const ACE_Time_Value &,
             {
               ORBSVCS_DEBUG ((LM_DEBUG,
                               ACE_TEXT ("(%P|%t) LiveCheck::handle_timeout(%d)")
-                              ACE_TEXT (", ping skipped for server <%C>\n"),
-                              token, entry->server_name ()));
+                              ACE_TEXT (", ping skipped for server <%C> may_ping <%d>\n"),
+                              token, entry->server_name (), entry->may_ping ()));
             }
         }
     }
diff --git a/TAO/orbsvcs/ImplRepo_Service/LiveCheck.h b/TAO/orbsvcs/ImplRepo_Service/LiveCheck.h
index fa9daf3c5ce..6437f7c289c 100644
--- a/TAO/orbsvcs/ImplRepo_Service/LiveCheck.h
+++ b/TAO/orbsvcs/ImplRepo_Service/LiveCheck.h
@@ -22,6 +22,11 @@
 #endif /* ACE_LACKS_PRAGMA_ONCE */
 
 #include "tao/Intrusive_Ref_Count_Handle_T.h"
+#if defined (ACE_HAS_CPP11)
+# include <atomic>
+#else
+# include "ace/Atomic_Op.h"
+#endif /* ACE_HAS_CPP11 */
 
 class LiveCheck;
 class LiveEntry;
@@ -81,7 +86,7 @@ class Locator_Export LiveListener
   /// is received. Returns true if finished listening
   virtual bool status_changed (LiveStatus status) = 0;
 
-  /// accessor for the server name. Used by the LiveCheck to associate a listener
+  /// Accessor for the server name. Used by the LiveCheck to associate a listener
   const char *server (void) const;
 
   LiveListener *_add_ref (void);
@@ -91,8 +96,11 @@ class Locator_Export LiveListener
   ACE_CString server_;
 
  private:
-  int refcount_;
-  TAO_SYNCH_MUTEX lock_;
+#if defined (ACE_HAS_CPP11)
+  std::atomic<int> refcount_;
+#else
+  ACE_Atomic_Op<TAO_SYNCH_MUTEX, int> refcount_;
+#endif /* ACE_HAS_CPP11 */
 };
 
 typedef TAO_Intrusive_Ref_Count_Handle<LiveListener> LiveListener_ptr;
@@ -140,6 +148,7 @@ class Locator_Export LiveEntry
   void set_pid (int pid);
   bool has_pid (int pid) const;
   int pid (void) const;
+  bool may_ping (void) const;
 
  private:
   LiveCheck *owner_;
diff --git a/TAO/orbsvcs/ImplRepo_Service/Locator_Options.cpp b/TAO/orbsvcs/ImplRepo_Service/Locator_Options.cpp
index bfd29ffd2b2..d691b4e8495 100644
--- a/TAO/orbsvcs/ImplRepo_Service/Locator_Options.cpp
+++ b/TAO/orbsvcs/ImplRepo_Service/Locator_Options.cpp
@@ -10,6 +10,7 @@
 #include "ace/Arg_Shifter.h"
 #include "orbsvcs/Log_Macros.h"
 #include "ace/OS_NS_strings.h"
+#include "ace/OS_NS_time.h"
 
 #if defined (ACE_WIN32)
 static const HKEY SERVICE_REG_ROOT = HKEY_LOCAL_MACHINE;
@@ -261,7 +262,7 @@ Options::parse_args (int &argc, ACE_TCHAR *argv[])
               return -1;
             }
           this->ping_interval_ =
-            ACE_Time_Value (0, 1000 * ACE_OS::atoi (shifter.get_current ()));
+            ACE_Time_Value (0, ACE_U_ONE_SECOND_IN_MSECS * ACE_OS::atoi (shifter.get_current ()));
         }
       else if (ACE_OS::strcasecmp (shifter.get_current (),
                                    ACE_TEXT ("-n")) == 0)
@@ -276,7 +277,7 @@ Options::parse_args (int &argc, ACE_TCHAR *argv[])
               return -1;
             }
           this->ping_timeout_ =
-            ACE_Time_Value (0, 1000 * ACE_OS::atoi (shifter.get_current ()));
+            ACE_Time_Value (0, ACE_U_ONE_SECOND_IN_MSECS * ACE_OS::atoi (shifter.get_current ()));
         }
       else if (ACE_OS::strcasecmp (shifter.get_current (),
                                    ACE_TEXT ("--ftendpoint")) == 0)
@@ -305,7 +306,7 @@ Options::parse_args (int &argc, ACE_TCHAR *argv[])
               return -1;
             }
           this->ft_update_delay_ =
-            ACE_Time_Value (0, 1000 * ACE_OS::atoi (shifter.get_current ()));
+            ACE_Time_Value (0, ACE_U_ONE_SECOND_IN_MSECS * ACE_OS::atoi (shifter.get_current ()));
         }
       else
         {
@@ -388,14 +389,16 @@ Options::print_usage (void) const
     ACE_TEXT ("  --backup        Replicate the ImplRepo as the backup ImR\n")
     ACE_TEXT ("  -r              Use the registry for storing/loading settings\n")
     ACE_TEXT ("  -s              Run as a service\n")
-    ACE_TEXT ("  -t secs         Server startup timeout.(Default = 60s)\n")
-    ACE_TEXT ("  -v msecs        Server verification interval.(Default = 10000ms)\n")
-    ACE_TEXT ("  -n msecs        Ping request timeout.(Default = 1000ms)\n")
+    ACE_TEXT ("  -t secs         Server startup timeout.(Default = %ds)\n")
+    ACE_TEXT ("  -v msecs        Server verification interval.(Default = %dms)\n")
+    ACE_TEXT ("  -n msecs        Ping request timeout.(Default = %dms)\n")
     ACE_TEXT ("  -i              Ping servers started without activators too.\n")
     ACE_TEXT ("  --lockout       Prevent excessive restart attempts until manual reset.\n")
     ACE_TEXT ("  --UnregisterIfAddressReused,\n")
-    ACE_TEXT ("  -u              Unregister server if its endpoint is used by another\n")
-              ));
+    ACE_TEXT ("  -u              Unregister server if its endpoint is used by another\n"),
+    DEFAULT_START_TIMEOUT,
+    DEFAULT_PING_INTERVAL * ACE_U_ONE_SECOND_IN_MSECS,
+    DEFAULT_PING_TIMEOUT * ACE_U_ONE_SECOND_IN_MSECS));
 }
 
 int
diff --git a/TAO/orbsvcs/tests/ImplRepo/Bug_4152_Regression/server.cpp b/TAO/orbsvcs/tests/ImplRepo/Bug_4152_Regression/server.cpp
index a3ff683ede1..45a56ca1902 100644
--- a/TAO/orbsvcs/tests/ImplRepo/Bug_4152_Regression/server.cpp
+++ b/TAO/orbsvcs/tests/ImplRepo/Bug_4152_Regression/server.cpp
@@ -8,9 +8,24 @@
 #include "tao/ImR_Client/ImR_Client.h"
 
 #include "ace/Get_Opt.h"
+#include "ace/Task.h"
 #include "ace/streams.h"
 #include "ace/OS_NS_unistd.h"
 
+class ORB_Runner : public ACE_Task_Base
+{
+public:
+  explicit ORB_Runner (CORBA::ORB_ptr orb) : orb_(CORBA::ORB::_duplicate(orb)) {}
+  int svc (void)
+  {
+    this->orb_->run ();
+    return 0;
+  }
+
+private:
+  CORBA::ORB_var orb_;
+};
+
 PortableServer::POA_var root_poa;
 PortableServer::POA_var poa_a;
 
@@ -46,6 +61,7 @@ int
 ACE_TMAIN (int argc, ACE_TCHAR *argv[])
 {
   CORBA::ORB_var orb = CORBA::ORB_init(argc, argv);
+  ORB_Runner *runner = new ORB_Runner (orb.in ());
   int poa_delay = 10;
   int shutdown_delay = 0;
 
@@ -79,6 +95,8 @@ ACE_TMAIN (int argc, ACE_TCHAR *argv[])
       CORBA::Object_var obj = orb->resolve_initial_references ("RootPOA");
       root_poa = PortableServer::POA::_narrow (obj.in ());
 
+      runner->activate ();
+
       ACE_CString base = ACE_CString ("TestObject");
       createPOAs (base);
 
@@ -120,10 +138,10 @@ ACE_TMAIN (int argc, ACE_TCHAR *argv[])
 
       test_ior = orb->object_to_string (tva.in());
       base += "_a";
-      ACE_DEBUG ((LM_DEBUG, "(%P|%t) %C: %C\n", base.c_str(), test_ior.in()));
+      ACE_DEBUG ((LM_DEBUG, "(%P|%t) %C:\n%C\n", base.c_str(), test_ior.in()));
       table->bind (base.c_str (), test_ior.in ());
 
-      orb->run ();
+      runner->wait ();
 
       ACE_DEBUG ((LM_DEBUG, "(%P|%t) Destroying POA pid <%P>\n"));
 
@@ -136,6 +154,7 @@ ACE_TMAIN (int argc, ACE_TCHAR *argv[])
       return 1;
     }
 
+  delete runner;
   orb = CORBA::ORB::_nil ();
 
   ACE_OS::sleep (shutdown_delay);