You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by James Peach <jp...@apache.org> on 2013/08/05 17:42:31 UTC

Re: git commit: [TS-2052] ET_SSL thread spinning

On Aug 2, 2013, at 5:43 PM, briang@apache.org wrote:

> Updated Branches:
>  refs/heads/master b91279d99 -> 94e9830c2
> 
> 
> [TS-2052] ET_SSL thread spinning
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/94e9830c
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/94e9830c
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/94e9830c
> 
> Branch: refs/heads/master
> Commit: 94e9830c21f046aafe0247fb9c95198fc0902144
> Parents: b91279d
> Author: Brian Geffon <br...@apache.org>
> Authored: Fri Aug 2 17:43:41 2013 -0700
> Committer: Brian Geffon <br...@apache.org>
> Committed: Fri Aug 2 17:43:41 2013 -0700
> 
> ----------------------------------------------------------------------
> CHANGES              | 4 +++-
> proxy/http/HttpSM.cc | 6 +++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/94e9830c/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index 3cc9cd7..c8fde03 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,8 @@
>                                                          -*- coding: utf-8 -*-
> Changes with Apache Traffic Server 3.3.5
> -
> +  
> +  *) [TS-2052] ET_SSL thread spinning
> +   Author: Can Selcik <cselcik at linkedin.com>

Next time, please use the --author option to attribute the author in the revision history.

Also it would be *really* helpful to commit some analysis of the bug and an explanation of the fix. This can go in as a commit log message, a source code comment, or even into the bug. This helps everyone understand the issues. 

> 
>   *) [TS-2090 Make proxy.config.allocator.enable_reclaim default based on
>    build instructions.
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/94e9830c/proxy/http/HttpSM.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
> index 5b879bc..7bba241 100644
> --- a/proxy/http/HttpSM.cc
> +++ b/proxy/http/HttpSM.cc
> @@ -741,7 +741,8 @@ HttpSM::state_read_client_request_header(int event, void *data)
>   }
> 
>   // Check to see if we are done parsing the header
> -  if (state != PARSE_CONT || ua_entry->eos) {
> +  if (state != PARSE_CONT || ua_entry->eos ||
> +	(state == PARSE_CONT && event == VC_EVENT_READ_COMPLETE)) {

This commit uses tabs instead of spaces :(

So if you get a READ_COMPLETE event while you are parsing the header, you reset the parser as if you had reached EOF, but you don't later abort the connection. What is the behaviour here?

>     if (ua_raw_buffer_reader != NULL) {
>         ua_raw_buffer_reader->dealloc();
>         ua_raw_buffer_reader = NULL;
> @@ -771,6 +772,9 @@ HttpSM::state_read_client_request_header(int event, void *data)
> 
>       call_transact_and_set_next_state(HttpTransact::BadRequest);
>       break;
> +    } else if (event == VC_EVENT_READ_COMPLETE) {
> +	DebugSM("http_parse", "[%" PRId64 "] VC_EVENT_READ_COMPLETE and PARSE CONT state", sm_id);
> +	break;
>     } else {
>       if (is_transparent_passthrough_allowed() &&
>           ua_raw_buffer_reader != NULL &&
>