Calling MICO::IIOPProxy::kill_conn can trigger memory race conditions

[Schneider, Wolfgang]

Hello MICO developers,

in my scenario a MICO service is called from a client process and delegating
those calls as several parallel invocations to further CORBA services. If
one of those delegation services is shut down, the service in the middle
could crash because of multiple deletes on the same IIOPProxyInvokeRec
object.
One of the delete candidates is the method IIOPProxy::del_invoke called in
IIOPProxy::abort_invoke itself. Another one is triggered as callback
function in CORBA::ORB::get_invoke_reply.


The following changes in MICO snapshot 2012 are my first try to fix the
problem (see the lines marked with // Wolfgang Schneider comments). Up to
now I didn't test that very heavily. So I'm very interested if you see any
side effects of my fix which I didn't had in mind.

Thanks a lot for any help.

Iop.cc:


void
MICO::IIOPProxy::abort_invoke (CORBA::ORBMsgId id)
{
// make invocation fail; notify orb ...
if (MICO::Logger::IsLogged (MICO::Logger::IIOP)) {
MICOMT::AutoDebugLock __lock;
MICO::Logger::Stream (MICO::Logger::IIOP)
<< "GIOP: invocation(" << id << ") aborted" << endl;
}

IIOPProxyInvokeRec* inv_rec = pull_invoke(id);

// Wolfgang Schneider, change 9.12.2013
// Do not delete the id object in invoke callback action
(CORBA::ORB::get_invoke_reply)
id->deactivate();

switch (_orb->request_type (id)) {
case CORBA::RequestInvoke: {
CORBA::Object_var obj = new CORBA::Object (new CORBA::IOR);
CORBA::Request_var req =
new CORBA::Request (obj, "someop");
LocalRequest orbreq (req);
orbreq.set_out_args (
new CORBA::TRANSIENT (0, CORBA::COMPLETED_MAYBE));
_orb->answer_invoke (id, CORBA::InvokeSysEx,
CORBA::Object::_nil(), &orbreq, 0);
break;
}
case CORBA::RequestLocate:
_orb->answer_locate (id, CORBA::LocateUnknown,
CORBA::Object::_nil(), 0);
break;

case CORBA::RequestBind:
_orb->answer_bind (id, CORBA::LocateUnknown,
CORBA::Object::_nil());
break;

default:
assert (0);
}

this->del_invoke(inv_rec);
}


Orb.cc:

CORBA::InvokeStatus
CORBA::ORB::get_invoke_reply (ORBMsgId id, Object_out obj, ORBRequest *&r,
GIOP::AddressingDisposition &ad)
{
ORBInvokeRec *rec = get_invoke (id);
assert (rec);

//Wolfgang Schneider, change 9.12.2013
//The id object will be deleted by IIOPProxy::abort_invoke
bool invoke_aborting = id->active() == false;

InvokeStatus state;
Object_ptr o;
CORBA::Boolean ret = rec->get_answer_invoke (state, o, r, ad);
assert (ret);
obj = Object::_duplicate (o);

//Wolfgang Schneider, change 9.12.2013
//The id object will be deleted by IIOPProxy::abort_invoke
//del_invoke ( rec->id() );
if ( !invoke_aborting ) {
del_invoke ( rec->id() );
}

#ifndef HAVE_THREADS
// XXX has to be changed for MT
//_currentid = 0;
if (!_currentid.empty()) {
_currentid.pop();
}
#else // HAVE_THREADS
stack<CORBA::ORBInvokeRec*>* invs =
static_cast<stack<CORBA::ORBInvokeRec*>*>
(MICOMT::Thread::get_specific(_current_rec_key));
if (invs != NULL && !invs->empty()) {
invs->pop();
}
#endif // HAVE_THREADS
return state;
}