[omniORB-dev] Multiple problems in OO 4.0.5 handling GIOP
CloseConnection messages
Jonathan Biggar
jon at levanta.com
Fri Apr 8 16:02:45 BST 2005
I have an omniORB server that needs to handle massive numbers of
clients, so it aggressively uses inConScanPeriod and outConScanPeriod
(since it is a client as well) to manage its connections. I've run into
a couple of issues with handling of the CloseConnection messages that
are generated by the server.
I've enclosed a large patch that fixes several issues:
1. If a client receives a CloseConnection message while waiting for a
Reply message, it doesn't recognize the graceful shutdown and throws
spurious COMM_FAILURE messages up the stack to the application.
I fixed this by having GIOP_C::notifyCommFailure test the strand's
orderly_closed flag, and if set, retry the same destination address
rather than moving on to the next one. This also required moving the
line of code that sets the orderly_closed flag in giopImpl1?.cc to
before the call to notifyCommFailure.
2. The GIOP server side state machine didn't keep track of the GIOP
version used by the client and would always send a CloseConnection
message using GIOP version 1.0. This caused spurious messages to be
generated by omniORB (and heartburn in other ORB implementations as
well). So this patch makes server side strands keep track of the GIOP
protocol version and use it when generating a CloseConnection message.
In addition, I made the GIOP 1.2 implementation more forgiving when
receiving a CloseConnection message with a version < 1.2.
I've attached the patch.
--
Jon Biggar
Levanta
jon at levanta.com
-------------- next part --------------
diff -c -r omniORB-4.0.5/src/lib/omniORB/orbcore/GIOP_C.cc.orig omniORB-4.0.5/src/lib/omniORB/orbcore/GIOP_C.cc
*** omniORB-4.0.5/src/lib/omniORB/orbcore/GIOP_C.cc.orig 2004-04-07 10:37:31.000000000 -0700
--- omniORB-4.0.5/src/lib/omniORB/orbcore/GIOP_C.cc 2005-03-08 11:24:56.000000000 -0800
***************
*** 269,286 ****
else {
currentaddr = pd_calldescriptor->currentAddress();
}
! currentaddr = pd_rope->notifyCommFailure(currentaddr,heldlock);
! pd_calldescriptor->currentAddress(currentaddr);
!
! if (currentaddr == firstaddr) {
! // Run out of addresses to try.
! retry = 0;
! pd_calldescriptor->firstAddressUsed(0);
! pd_calldescriptor->currentAddress(0);
! }
! else {
! // Retry will use the next address in the list.
retry = 1;
}
}
else if (pd_strand->biDir &&
--- 269,290 ----
else {
currentaddr = pd_calldescriptor->currentAddress();
}
! if (pd_strand->orderly_closed) {
retry = 1;
+ } else {
+ currentaddr = pd_rope->notifyCommFailure(currentaddr,heldlock);
+ pd_calldescriptor->currentAddress(currentaddr);
+
+ if (currentaddr == firstaddr) {
+ // Run out of addresses to try.
+ retry = 0;
+ pd_calldescriptor->firstAddressUsed(0);
+ pd_calldescriptor->currentAddress(0);
+ }
+ else {
+ // Retry will use the next address in the list.
+ retry = 1;
+ }
}
}
else if (pd_strand->biDir &&
diff -c -r omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl10.cc.orig omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl10.cc
*** omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl10.cc.orig 2004-07-01 12:16:25.000000000 -0700
--- omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl10.cc 2005-03-08 11:32:13.000000000 -0800
***************
*** 287,295 ****
{
CORBA::ULong minor;
CORBA::Boolean retry;
g->notifyCommFailure(0,minor,retry);
g->pd_strand->state(giopStrand::DYING);
- g->pd_strand->orderly_closed = 1;
giopStream::CommFailure::_raise(minor,
CORBA::COMPLETED_NO,
retry,__FILE__,__LINE__);
--- 287,295 ----
{
CORBA::ULong minor;
CORBA::Boolean retry;
+ g->pd_strand->orderly_closed = 1;
g->notifyCommFailure(0,minor,retry);
g->pd_strand->state(giopStrand::DYING);
giopStream::CommFailure::_raise(minor,
CORBA::COMPLETED_NO,
retry,__FILE__,__LINE__);
diff -c -r omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl11.cc.orig omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl11.cc
*** omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl11.cc.orig 2004-07-01 12:16:24.000000000 -0700
--- omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl11.cc 2005-03-08 11:32:01.000000000 -0800
***************
*** 232,237 ****
--- 232,238 ----
// This is a GIOP 1.0 message, switch to the implementation of giop 1.0
// and dispatch again.
GIOP::Version v = { 1, 0 };
+ ((giopStrand &)*g).version = v;
g->impl(giopStreamImpl::matchVersion(v));
OMNIORB_ASSERT(g->impl());
g->impl()->inputMessageBegin(g,g->impl()->unmarshalWildCardRequestHeader);
***************
*** 300,308 ****
{
CORBA::ULong minor;
CORBA::Boolean retry;
g->notifyCommFailure(0,minor,retry);
g->pd_strand->state(giopStrand::DYING);
- g->pd_strand->orderly_closed = 1;
giopStream::CommFailure::_raise(minor,
CORBA::COMPLETED_NO,
retry,__FILE__,__LINE__);
--- 301,309 ----
{
CORBA::ULong minor;
CORBA::Boolean retry;
+ g->pd_strand->orderly_closed = 1;
g->notifyCommFailure(0,minor,retry);
g->pd_strand->state(giopStrand::DYING);
giopStream::CommFailure::_raise(minor,
CORBA::COMPLETED_NO,
retry,__FILE__,__LINE__);
diff -c -r omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl12.cc.orig omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl12.cc
*** omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl12.cc.orig 2004-10-17 17:23:54.000000000 -0700
--- omniORB-4.0.5/src/lib/omniORB/orbcore/giopImpl12.cc 2005-03-08 13:17:39.000000000 -0800
***************
*** 233,240 ****
// rather the equivalent place in GIOP 1.3+) due to receiving a
// reply message on the receiver thread of a bidirectional
// connection.
! inputTerminalProtocolError(g, __FILE__, __LINE__);
! // never reach here
}
--- 233,244 ----
// rather the equivalent place in GIOP 1.3+) due to receiving a
// reply message on the receiver thread of a bidirectional
// connection.
!
! // We accept a CloseConnection message with any GIOP version.
! if ((GIOP::MsgType)hdr[7] != GIOP::CloseConnection) {
! inputTerminalProtocolError(g, __FILE__, __LINE__);
! // never reach here
! }
}
***************
*** 260,268 ****
// orderly shutdown.
CORBA::ULong minor;
CORBA::Boolean retry;
g->notifyCommFailure(0,minor,retry);
g->pd_strand->state(giopStrand::DYING);
- g->pd_strand->orderly_closed = 1;
giopStream::CommFailure::_raise(minor,
CORBA::COMPLETED_NO,
retry,__FILE__,__LINE__);
--- 264,272 ----
// orderly shutdown.
CORBA::ULong minor;
CORBA::Boolean retry;
+ g->pd_strand->orderly_closed = 1;
g->notifyCommFailure(0,minor,retry);
g->pd_strand->state(giopStrand::DYING);
giopStream::CommFailure::_raise(minor,
CORBA::COMPLETED_NO,
retry,__FILE__,__LINE__);
***************
*** 700,705 ****
--- 704,710 ----
GIOP::Version v;
v.major = 1;
v.minor = hdr[5];
+ ((giopStrand &)*g).version = v;
g->impl(giopStreamImpl::matchVersion(v));
OMNIORB_ASSERT(g->impl());
g->impl()->inputMessageBegin(g,g->impl()->unmarshalWildCardRequestHeader);
diff -c -r omniORB-4.0.5/src/lib/omniORB/orbcore/giopServer.cc.orig omniORB-4.0.5/src/lib/omniORB/orbcore/giopServer.cc
*** omniORB-4.0.5/src/lib/omniORB/orbcore/giopServer.cc.orig 2004-10-17 13:14:32.000000000 -0700
--- omniORB-4.0.5/src/lib/omniORB/orbcore/giopServer.cc 2005-03-08 20:46:44.000000000 -0800
***************
*** 645,650 ****
--- 645,651 ----
giopStrand* s = new giopStrand(conn,this);
+ s->version.major = 1; s->version.minor = 0;
{
ASSERT_OMNI_TRACEDMUTEX_HELD(*omniTransportLock,0);
omni_tracedmutex_lock sync(*omniTransportLock);
diff -c -r omniORB-4.0.5/src/lib/omniORB/orbcore/giopStrand.cc.orig omniORB-4.0.5/src/lib/omniORB/orbcore/giopStrand.cc
*** omniORB-4.0.5/src/lib/omniORB/orbcore/giopStrand.cc.orig 2004-10-17 14:48:40.000000000 -0700
--- omniORB-4.0.5/src/lib/omniORB/orbcore/giopStrand.cc 2005-03-08 14:09:09.000000000 -0800
***************
*** 467,473 ****
sp->state(IOP_S::Idle);
if (!sp->impl()) {
! sp->impl(giopStreamImpl::maxVersion());
}
// the codeset convertors are filled in by the codeset interceptor
// before a request is unmarshalled.
--- 467,476 ----
sp->state(IOP_S::Idle);
if (!sp->impl()) {
! giopStreamImpl *impl = giopStreamImpl::maxVersion();
! sp->impl(impl);
! if (version.major == 0)
! version = impl->version();
}
// the codeset convertors are filled in by the codeset interceptor
// before a request is unmarshalled.
***************
*** 727,736 ****
}
{
// Send close connection message.
- GIOP::Version ver = giopStreamImpl::maxVersion()->version();
char hdr[12];
hdr[0] = 'G'; hdr[1] = 'I'; hdr[2] = 'O'; hdr[3] = 'P';
! hdr[4] = ver.major; hdr[5] = ver.minor;
hdr[6] = _OMNIORB_HOST_BYTE_ORDER_;
hdr[7] = (char)GIOP::CloseConnection;
hdr[8] = hdr[9] = hdr[10] = hdr[11] = 0;
--- 730,738 ----
}
{
// Send close connection message.
char hdr[12];
hdr[0] = 'G'; hdr[1] = 'I'; hdr[2] = 'O'; hdr[3] = 'P';
! hdr[4] = s->version.major; hdr[5] = s->version.minor;
hdr[6] = _OMNIORB_HOST_BYTE_ORDER_;
hdr[7] = (char)GIOP::CloseConnection;
hdr[8] = hdr[9] = hdr[10] = hdr[11] = 0;
***************
*** 1010,1019 ****
}
{
// Send close connection message.
- GIOP::Version ver = giopStreamImpl::maxVersion()->version();
char hdr[12];
hdr[0] = 'G'; hdr[1] = 'I'; hdr[2] = 'O'; hdr[3] = 'P';
! hdr[4] = ver.major; hdr[5] = ver.minor;
hdr[6] = _OMNIORB_HOST_BYTE_ORDER_;
hdr[7] = (char)GIOP::CloseConnection;
hdr[8] = hdr[9] = hdr[10] = hdr[11] = 0;
--- 1012,1020 ----
}
{
// Send close connection message.
char hdr[12];
hdr[0] = 'G'; hdr[1] = 'I'; hdr[2] = 'O'; hdr[3] = 'P';
! hdr[4] = s->version.major; hdr[5] = s->version.minor;
hdr[6] = _OMNIORB_HOST_BYTE_ORDER_;
hdr[7] = (char)GIOP::CloseConnection;
hdr[8] = hdr[9] = hdr[10] = hdr[11] = 0;
More information about the omniORB-dev
mailing list