Bug 2904 - Servant_Activator
Summary: Servant_Activator
Status: RESOLVED FIXED
Alias: None
Product: CIAO
Classification: Unclassified
Component: CIAO Container Implementation (show other bugs)
Version: 0.5.7
Hardware: All All
: P3 normal
Assignee: Will Otte
URL:
Depends on: 2903
Blocks: 3253 3576
  Show dependency tree
 
Reported: 2007-04-20 12:05 CDT by Abdul Sowayan
Modified: 2009-08-10 13:40 CDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Abdul Sowayan 2007-04-20 12:05:55 CDT
Consider the following code from Servant_Activator.cpp 

  PortableServer::Servant
  Servant_Activator::incarnate (const PortableServer::ObjectId &oid,
                                PortableServer::POA_ptr)
  {
    CORBA::String_var str =
      PortableServer::ObjectId_to_string (oid);

    if (CIAO::debug_level () > 9)
      ACE_DEBUG ((LM_DEBUG,
                  "CIAO (%P|%t) - Servant_Activator::incarnate, "
                  "activating port name [%s] \n",
                  str.in ()));

    {
      // i removed some code to just illucidate my point
      // we first acquire the mutex via ACE_Guard

      ACE_GUARD_RETURN (TAO_SYNCH_MUTEX,
                        guard,
                        this->mutex_,
                        0);

      for (size_t t = 0; t != sz; ++t)
        {
          if (ACE_OS::strcmp (tmp->oid (),
                              str.in ()) == 0)
            {
              // We should try avoiding making outbound calls with the
              // lock held. Oh well, let us get some sense of sanity in
              // CIAO to do think about these.
              if (CIAO::debug_level () > 5)
                ACE_DEBUG ((LM_DEBUG, "Activating Port %s\n",
                            str.in ()));

              return this->pa_[t]->activate (oid);
            }
        }
    }
  }

Well, if we don't need to hold the lock before we make the outbound call, why are we doing it? Can't we do something like this instead?

if (ACE_OS::strcmp (tmp->oid (),
                    str.in ()) == 0)
{
  // first obtain the pointer to Port_Activator
  // with the mutex held
  Port_Activator* pa = this->pa_[t];

  // now release the mutex
  // when we release the mutex like this
  // it will not be released again when the scope
  // of the guard ends. Check ACE_Guard implementation
  // and bugzilla 2903 discussion.
  guard.release() 

  // now activate, we're not holding the lock
  // on an outbound call.
  return this->pa_[t]->activate (oid);
}
Comment 1 Johnny Willemsen 2007-04-25 13:57:09 CDT
changed severity
Comment 2 Will Otte 2009-08-10 13:40:08 CDT
Fix recently merged to head.  We no longer hold the lock on the outbound call.