[Ace-users] [tao-bugs] Bug in MT_NOUPCALL prevents upcall disabling

[Robert Hancock]

Back in 2005, Mickael P. Golovin pointed out a problem in the 
LF_NOUPCALL client connection handler, where it failed to disable 
upcalls in some cases:

http://groups.google.ca/group/comp.soft-sys.ace/msg/868295873678066f

Looking at the current code it appears that this fix is still needed. 
With the current code, upcalls are only disabled if the transport was 
opened in the client role and is not bidirectional. However in this case 
upcalls on that connection are not even possible so this makes no sense. 
Previously the upcalls were disabled if the transport was opened in the 
server role or is bidirectional, which makes more sense.

The code was originally correct but was broken by the change below.

Sun Oct  3 13:38:01 2004  Balachandran Natarajan  <bala at dre.vanderbilt.edu>

         * tao/Wait_Strategy.h:

           Added a new pure virtual method, can_process_upcalls ().

         * tao/Wait_On_LF_No_Upcall.cpp:
         * tao/Wait_On_LF_No_Upcall.h:
         * tao/Wait_On_Leader_Follower.cpp:
         * tao/Wait_On_Leader_Follower.h:
         * tao/Wait_On_Reactor.cpp:
         * tao/Wait_On_Reactor.h:
         * tao/Wait_On_Read.cpp:
         * tao/Wait_On_Read.h:

           Implementation for can_process_upcalls (). This method returns
           true for all the Wait strategies but for
           Wait_On_LF_No_Upcall. Only the Wait_On_LF_No_Upcall returns
           false since the semantics of the class are so.

         * tao/Connection_Handler.cpp:

           Use the can_process_upcalls () in handle_input_eh () instead of
           the checks in place there. This check invariably resulted in a
           TSS access which seem to slow down things quite a bit. This was
           based on my profiler. I am not sure whether this is going to
           show up any conclusive results in our stat pages.

         * tao/Transport.h:
         * tao/Transport.inl:

           Removed the acts_as_server () call since the functionality is
           replaced by the new pure virtual method in Wait_Strategy.

I've made a patch based on their suggested fix, it's below. It seems 
like this fix should be applied as otherwise MT_NOUPCALL will basically 
not work at all..

--- ACE_wrappers/TAO/tao/Wait_On_LF_No_Upcall.cpp.orig  2008-02-25 
20:02:52.000000000 +0000
+++ ACE_wrappers/TAO/tao/Wait_On_LF_No_Upcall.cpp       2008-02-25 
20:08:10.000000000 +0000
@@ -96,13 +96,15 @@ namespace TAO
    bool
    Wait_On_LF_No_Upcall::can_process_upcalls (void) const
    {
-    TAO_ORB_Core_TSS_Resources *tss =
-      this->transport_->orb_core ()->get_tss_resources ();
+    if ((this->transport_->opened_as () == TAO::TAO_SERVER_ROLE) ||
+        (this->transport_->bidirectional_flag () == 1))
+      {
+        TAO_ORB_Core_TSS_Resources *tss =
+          this->transport_->orb_core ()->get_tss_resources ();

-    if ((this->transport_->opened_as () == TAO::TAO_CLIENT_ROLE) &&
-        (this->transport_->bidirectional_flag () == 0) &&
-        (tss->upcalls_temporarily_suspended_on_this_thread_ == true))
-      return false;
+        if (tss->upcalls_temporarily_suspended_on_this_thread_ == true)
+          return false;
+      }

      return true;
    }



-- 
Robert Hancock
Programmer Analyst
SED Systems
Email: hancock at sedsystems.ca