You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2014/11/19 18:47:32 UTC
trafficserver git commit: TS-3189: Delay the do_io_read on the server
to user agent tunnel to avoid cases of the incorrect tunnel handling EOS in
the post case. This closes #143.
Repository: trafficserver
Updated Branches:
refs/heads/master deb77cf15 -> a34bebbe5
TS-3189: Delay the do_io_read on the server to user agent tunnel to avoid
cases of the incorrect tunnel handling EOS in the post case.
This closes #143.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/a34bebbe
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/a34bebbe
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/a34bebbe
Branch: refs/heads/master
Commit: a34bebbe5bf432c9d27052a31c68dadef46e32c7
Parents: deb77cf
Author: shinrich <sh...@network-geographics.com>
Authored: Tue Nov 11 11:24:13 2014 -0600
Committer: Alan M. Carroll <so...@yahoo-inc.com>
Committed: Wed Nov 19 11:47:10 2014 -0600
----------------------------------------------------------------------
CHANGES | 2 ++
proxy/http/HttpSM.cc | 62 +++++++++++++++--------------------------------
2 files changed, 22 insertions(+), 42 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a34bebbe/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index a56c126..7f21e35 100644
--- a/CHANGES
+++ b/CHANGES
@@ -3,6 +3,8 @@ Changes with Apache Traffic Server 5.2.0
*) [TS-3188] Do not set half_close_flag on keep_alive connections.
+ *) [TS-3189] Delay starting read on server to user agent tunnel.
+
*) [TS-2417] Add forward secrecy support with DHE.
Author: John Eaglesham <je...@8192.net>
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a34bebbe/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 413e3df..f48b5f1 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -575,7 +575,7 @@ HttpSM::attach_client_session(HttpClientSession * client_vc, IOBufferReader * bu
// 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, NULL);
/////////////////////////
// set up timeouts //
@@ -5645,18 +5645,19 @@ HttpSM::attach_server_session(HttpServerSession * s)
// do the read with a buffer and a size so preallocate the
// buffer
server_buffer_reader = server_session->get_reader();
- server_entry->read_vio = server_session->do_io_read(this, INT64_MAX, server_session->read_buffer);
-
- // This call cannot be canceled or disabled on Windows at a different
- // time (callstack). After this function, all transactions will send
- // a request to the origin server. It is possible that read events
- // for the response come in before the write events for sending the
- // request itself. In state_send_server_request(), we try to disable
- // reading until writing the request completed. That turned out to be
- // for the second do_io_read(), the way to reenable() reading once
- // disabled, but still the result of this do_io_read came in. For this
- // read holds: server_entry->read_vio == INT64_MAX
- // This block of read events gets undone in setup_server_read_response()
+ // ts-3189 We are only setting up an empty read at this point. This
+ // is suffient to have the timeout errors directed to the appropriate
+ // SM handler, but we don't want to read any data until the tunnel has
+ // been set up. This isn't such a big deal with GET results, since
+ // if no tunnels are set up, there is no danger of data being delivered
+ // to the wrong tunnel's consumer handler. But for post and other
+ // methods that send data after the request, two tunnels are created in
+ // series, and with a full read set up at this point, the EOS from the
+ // first tunnel was sometimes behind handled by the consumer of the
+ // first tunnel instead of the producer of the second tunnel.
+ // The real read is setup in setup_server_read_response_header()
+ //
+ server_entry->read_vio = server_session->do_io_read(this, 0, NULL);
// Transfer control of the write side as well
server_session->do_io_write(this, 0, NULL);
@@ -5784,40 +5785,17 @@ HttpSM::setup_server_read_response_header()
// read the request header
ink_assert(server_entry->read_vio);
+ // The tunnel from OS to UA is now setup. Ready to read the response
+ server_entry->read_vio = server_session->do_io_read(this, INT64_MAX, server_buffer_reader->mbuf);
+
// If there is anything in the buffer call the parsing routines
// since if the response is finished, we won't get any
// additional callbacks
- //UnixNetVConnection * vc = (UnixNetVConnection*)(ua_session->client_vc);
- //UnixNetVConnection * my_server_vc = (UnixNetVConnection*)(server_session->get_netvc());
if (server_buffer_reader->read_avail() > 0) {
- if (server_entry->eos) {
- /*printf("data already in the buffer, calling state_read_server_response_header with VC_EVENT_EOS client fd: %d and server fd : %d\n",
- vc->con.fd,my_server_vc->con.fd); */
- state_read_server_response_header(VC_EVENT_EOS, server_entry->read_vio);
- } else {
- /*printf("data alreadyclient_vc in the buffer, calling state_read_server_response_header with VC_EVENT_READ_READY fd: %d and server fd : %d\n",
- vc->con.fd,my_server_vc->con.fd); */
- state_read_server_response_header(VC_EVENT_READ_READY, server_entry->read_vio);
- }
- }
- // It is possible the header was already in the buffer and the
- // IO on for read disabled. This would happen if 1) the response
- // header was received before the 'request sent' callback happened
- // (or before a post body was sent) OR 2) we were parsing a 100
- // continue response and are now parsing next response header
- // If only part of the header was in the buffer we need to do more IO
- // to get the rest of it. If the whole header is in the buffer then we don't
- // want additional IO since we'll be issuing another read for body tunnel
- // and can't switch buffers at that point since we won't be on the read
- // callback
- if (server_entry != NULL) {
- if (t_state.current.server->state == HttpTransact::STATE_UNDEFINED &&
- server_entry->read_vio->nbytes == server_entry->read_vio->ndone &&
- milestones.server_read_header_done == 0) {
- ink_assert(server_entry->eos == false);
- server_entry->read_vio = server_session->do_io_read(this, INT64_MAX, server_buffer_reader->mbuf);
- }
+ state_read_server_response_header(
+ (server_entry->eos) ? VC_EVENT_EOS : VC_EVENT_READ_READY,
+ server_entry->read_vio);
}
}