Great !
Can you implement and send diffs? Perhaps this is too big a change at this late stage in 1.5 beta - thoughts anyone?
| Tim Bartley <[EMAIL PROTECTED]>
11/03/2005 23:07
|
|
HTTP 1.1 (and 1.0 with Connection: Keep-Alive) permits the client to re-use a connection for multiple requests and Axis makes use of this.
However, if the client hasn't sent a request on that connection for a while the server will typlically shutdown the connection.
One server I've seen (WebSphere) does this simply by sending a FIN from it's end. This means that the client->server half of the connection is still open so the next client send (*m_pActiveChannel << this->getHTTPHeaders() in HTTPTransport::flushOutput) succeeds. The server responds to this by aborting the connection but it's not until the the next send (*m_pActiveChannel << this->m_strBytesToSend.c_str()) that an IO error occurs and ultimately an exception is thrown to the client application.
Now this behaviour is a property of the transport so I think Axis should detect that the server side has closed the connection and resend the request. This should always be OK because an IO error on any part of the send must mean the request has not been completely sent and therefore re-sending the request should not be harmful.
So I think HTTPTransport::flushOutput should have some logic like:
try {
*m_pActiveChannel << this->getHTTPHeaders();
*m_pActiveChannel << this->strBytesToSend.c_str();
}
catch (HTTPTransportException& e) {
if (didn't just re-open the connection) {
m_pActiveChannel->close();
m_pActiveChannel->open();
*m_pActiveChannel << this->getHTTPHeaders();
*m_pActiveChannel << this->strBytesToSend.c_str();
}
else {
throw;
}
}
We can do slightly better than this by trying to detect that the server has closed the connection before sending at all - this saves the network bandwidth of the first packet and saves us a bit of computation. This can be done approximately as follows:
bool reopenConnection;
fd_set_t read_fds;
fd_set_t except_fds;
FD_ZERO(&read_fds);
FD_ZERO(&except_fds);
FD_SET(socket, &read_fds);
FD_SET(socket, &except_fds);
timepec_t t = {0};
int result = select(FD_SETSIZE, &read_fds, NULL, &except_fds, &t);
if (result < 0) {
throw something;
}
else if (result == 0) {
/* socket not readable - therefore not closed - ok to send */
reopenConnection = false;
}
else {
/* socket readable or in error - see if data available */
unsigned char byte;
result = recv(socket, &byte, 1, MSG_PEEK);
if (result == 0) {
/* socket shutdown by remote end */
reopenConnection = true;
}
else if (result < 0) {
if (errno == ECONNRESET) {
reopenConnection = true;
}
else {
/* Possibly this is too aggressive and reopenConnection should be set to true irrespective of the errno value */
throw something;
}
}
}
return reopenConnection
I suggest the above logic be encapsulated in the channels and accessed through the IChannel interface in flushOutput as something like:
bool connectionJustReopened = false;
if (m_bReopenConnection || m_pActiveChannel->connectionReopenRequired()) {
m_pActiveChannel->close();
m_pActiveChannel->open();
connectionJustReopened = true;
}
bool retry;
do {
retry = false;
try {
*m_pActiveChannel << this->getHTTPHeaders();
*m_pActiveChannel << this->strDataBytes.c_str();
}
catch (HTTPTransportException& e) {
if (!connectionJustReopened) {
m_pActiveChannel->close();
m_pActiveChannel->open();
retry = true;
connectionJustReopened = true;
}
else {
throw;
}
}
} while (retry);
Even if we implement a connectionReopenRequired interface we still need to re-open on IO error from the send because there is a race condition between when we test this and actually send the request - the connection ReopenRequired interface is really just an optimization.
What do you think?
Cheers,
Tim
--
IBM Tivoli Access Manager Development
Gold Coast Development Lab, Australia
+61-7-5552-4001 phone
+61-7-5571-0420 fax
