You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2020/06/09 22:35:19 UTC
[trafficserver] branch master updated: Avoid stale client_vc (#6732)
This is an automated email from the ASF dual-hosted git repository.
shinrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new aca1436 Avoid stale client_vc (#6732)
aca1436 is described below
commit aca14366a74a56979b05404f54f4a099d4c20d76
Author: Susan Hinrichs <sh...@yahoo-inc.com>
AuthorDate: Tue Jun 9 17:35:03 2020 -0500
Avoid stale client_vc (#6732)
* Avoid stale client_vc
* Avoid write vios
Co-authored-by: Susan Hinrichs <sh...@verizonmedia.com>
---
iocore/net/UnixNetVConnection.cc | 2 ++
proxy/http/Http1ClientSession.cc | 24 +++++++-----------------
proxy/http/HttpSM.cc | 12 ++++++------
proxy/http2/Http2ClientSession.cc | 8 ++------
proxy/http2/Http2Stream.cc | 5 +++--
5 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 181420e..dc112a6 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -88,6 +88,7 @@ read_signal_and_update(int event, UnixNetVConnection *vc)
case VC_EVENT_ACTIVE_TIMEOUT:
case VC_EVENT_INACTIVITY_TIMEOUT:
Debug("inactivity_cop", "event %d: null read.vio cont, closing vc %p", event, vc);
+ Warning("read: Closing orphaned vc %p", vc);
vc->closed = 1;
break;
default:
@@ -119,6 +120,7 @@ write_signal_and_update(int event, UnixNetVConnection *vc)
case VC_EVENT_ACTIVE_TIMEOUT:
case VC_EVENT_INACTIVITY_TIMEOUT:
Debug("inactivity_cop", "event %d: null write.vio cont, closing vc %p", event, vc);
+ Warning("write: Closing orphaned vc %p", vc);
vc->closed = 1;
break;
default:
diff --git a/proxy/http/Http1ClientSession.cc b/proxy/http/Http1ClientSession.cc
index a998a80..a289847 100644
--- a/proxy/http/Http1ClientSession.cc
+++ b/proxy/http/Http1ClientSession.cc
@@ -111,9 +111,6 @@ Http1ClientSession::free()
conn_decrease = false;
}
- // Clean up the write VIO in case of inactivity timeout
- this->do_io_write(nullptr, 0, nullptr);
-
// Free the transaction resources
this->trans.super_type::destroy();
@@ -228,12 +225,13 @@ Http1ClientSession::do_io_close(int alerrno)
slave_ka_vio = nullptr;
}
// Completed the last transaction. Just shutdown already
- if (transact_count == released_transactions) {
+ // Or the do_io_close is due to a network error
+ if (transact_count == released_transactions || alerrno == HTTP_ERRNO) {
half_close = false;
}
// Clean up the write VIO in case of inactivity timeout
- this->do_io_write(nullptr, 0, nullptr);
+ this->do_io_write(this, 0, nullptr);
if (half_close && this->trans.get_sm()) {
read_state = HCS_HALF_CLOSED;
@@ -293,11 +291,7 @@ Http1ClientSession::state_wait_for_close(int event, void *data)
case VC_EVENT_ACTIVE_TIMEOUT:
case VC_EVENT_INACTIVITY_TIMEOUT:
half_close = false;
- this->do_io_close();
- if (_vc != nullptr) {
- _vc->do_io_close();
- _vc = nullptr;
- }
+ this->do_io_close(EHTTP_ERROR);
break;
case VC_EVENT_READ_READY:
// Drain any data read
@@ -373,11 +367,7 @@ Http1ClientSession::state_keep_alive(int event, void *data)
break;
case VC_EVENT_EOS:
- this->do_io_close();
- if (_vc != nullptr) {
- _vc->do_io_close();
- _vc = nullptr;
- }
+ this->do_io_close(EHTTP_ERROR);
break;
case VC_EVENT_READ_COMPLETE:
@@ -389,7 +379,7 @@ Http1ClientSession::state_keep_alive(int event, void *data)
case VC_EVENT_ACTIVE_TIMEOUT:
case VC_EVENT_INACTIVITY_TIMEOUT:
// Keep-alive timed out
- this->do_io_close();
+ this->do_io_close(EHTTP_ERROR);
break;
}
@@ -403,7 +393,7 @@ Http1ClientSession::release(ProxyTransaction *trans)
ink_assert(read_state == HCS_ACTIVE_READER || read_state == HCS_INIT);
// Clean up the write VIO in case of inactivity timeout
- this->do_io_write(nullptr, 0, nullptr);
+ this->do_io_write(this, 0, nullptr);
// Check to see there is remaining data in the
// buffer. If there is, spin up a new state
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 6dfbe23..955fd74 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -583,7 +583,8 @@ HttpSM::attach_client_session(ProxyTransaction *client_vc, IOBufferReader *buffe
// this hook maybe asynchronous, we need to disable IO on
// client but set the continuation to be the state machine
// so if we get an timeout events the sm handles them
- ua_entry->read_vio = client_vc->do_io_read(this, 0, buffer_reader->mbuf);
+ ua_entry->read_vio = client_vc->do_io_read(this, 0, buffer_reader->mbuf);
+ ua_entry->write_vio = client_vc->do_io_write(this, 0, nullptr);
/////////////////////////
// set up timeouts //
@@ -2764,7 +2765,7 @@ HttpSM::tunnel_handler_post(int event, void *data)
if (ua_entry->write_buffer) {
free_MIOBuffer(ua_entry->write_buffer);
ua_entry->write_buffer = nullptr;
- ua_entry->vc->do_io_write(nullptr, 0, nullptr);
+ ua_entry->vc->do_io_write(this, 0, nullptr);
}
if (!p->handler_state) {
p->handler_state = HTTP_SM_POST_UA_FAIL;
@@ -3511,7 +3512,7 @@ HttpSM::tunnel_handler_post_ua(int event, HttpTunnelProducer *p)
// if it is active timeout case, we need to give another chance to send 408 response;
ua_txn->set_active_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_active_timeout_in));
- p->vc->do_io_write(nullptr, 0, nullptr);
+ p->vc->do_io_write(this, 0, nullptr);
p->vc->do_io_shutdown(IO_SHUTDOWN_READ);
return 0;
@@ -3525,7 +3526,7 @@ HttpSM::tunnel_handler_post_ua(int event, HttpTunnelProducer *p)
// server and close the ua
p->handler_state = HTTP_SM_POST_UA_FAIL;
set_ua_abort(HttpTransact::ABORTED, event);
- p->vc->do_io_write(nullptr, 0, nullptr);
+ p->vc->do_io_write(this, 0, nullptr);
p->vc->do_io_shutdown(IO_SHUTDOWN_READ);
tunnel.chain_abort_all(p);
server_session = nullptr;
@@ -3553,8 +3554,7 @@ HttpSM::tunnel_handler_post_ua(int event, HttpTunnelProducer *p)
}
}
- // Initiate another read to watch catch aborts and
- // timeouts
+ // Initiate another read to catch aborts and timeouts.
ua_entry->vc_handler = &HttpSM::state_watch_for_client_abort;
ua_entry->read_vio = p->vc->do_io_read(this, INT64_MAX, ua_buffer_reader->mbuf);
break;
diff --git a/proxy/http2/Http2ClientSession.cc b/proxy/http2/Http2ClientSession.cc
index 361f2f8..eedafa0 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -282,8 +282,8 @@ Http2ClientSession::do_io_close(int alerrno)
this->clear_session_active();
- // Clean up the write VIO in case of inactivity timeout
- this->do_io_write(nullptr, 0, nullptr);
+ // Point the write_vio at the session in case of inactivity timeout
+ this->do_io_write(this, 0, nullptr);
}
void
@@ -345,10 +345,6 @@ Http2ClientSession::main_event_handler(int event, void *edata)
case VC_EVENT_EOS:
this->set_dying_event(event);
this->do_io_close();
- if (_vc != nullptr) {
- _vc->do_io_close();
- _vc = nullptr;
- }
retval = 0;
break;
diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc
index 38e2acd..5a71250 100644
--- a/proxy/http2/Http2Stream.cc
+++ b/proxy/http2/Http2Stream.cc
@@ -663,7 +663,8 @@ Http2Stream::signal_read_event(int event)
void
Http2Stream::signal_write_event(int event)
{
- if (this->write_vio.cont == nullptr || this->write_vio.cont->mutex == nullptr || this->write_vio.op == VIO::NONE) {
+ if (this->write_vio.cont == nullptr || this->write_vio.cont->mutex == nullptr || this->write_vio.op == VIO::NONE ||
+ this->write_vio.nbytes == 0) {
return;
}
@@ -686,7 +687,7 @@ Http2Stream::signal_write_event(bool call_update)
return;
}
- if (this->write_vio.get_writer()->write_avail() == 0) {
+ if (this->write_vio.get_writer()->write_avail() == 0 || this->write_vio.nbytes == 0) {
return;
}