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 2014/07/08 21:38:39 UTC

Re: git commit: TS-2780: Core dump in SpdyRequest::clear() in production testing of SPDY

A helper function would make this cleaner:

SpdyRequest *
SpdyClientSession::find_request(int streamid) {
  map<int32_t, SpdyRequest*>::iterator iter = this->req_map.find(streamid);
  return iter == this->req_map.end() ? NULL : iter->second;
}

On Jul 8, 2014, at 12:29 PM, bcall@apache.org wrote:

> Repository: trafficserver
> Updated Branches:
>  refs/heads/master 0c18f7bcd -> c9d443353
> 
> 
> TS-2780: Core dump in SpdyRequest::clear() in production testing of SPDY
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/c9d44335
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/c9d44335
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/c9d44335
> 
> Branch: refs/heads/master
> Commit: c9d4433531767c9b5f6db42b488af4552bc5c4a9
> Parents: 0c18f7b
> Author: Sudheer Vinukonda <su...@yahoo-inc.com>
> Authored: Tue Jul 8 12:28:57 2014 -0700
> Committer: Bryan Call <bc...@apache.org>
> Committed: Tue Jul 8 12:28:57 2014 -0700
> 
> ----------------------------------------------------------------------
> proxy/spdy/SpdyCallbacks.cc     | 10 ++++++----
> proxy/spdy/SpdyClientSession.cc |  3 ++-
> 2 files changed, 8 insertions(+), 5 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c9d44335/proxy/spdy/SpdyCallbacks.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/spdy/SpdyCallbacks.cc b/proxy/spdy/SpdyCallbacks.cc
> index 5a0564f..830c336 100644
> --- a/proxy/spdy/SpdyCallbacks.cc
> +++ b/proxy/spdy/SpdyCallbacks.cc
> @@ -372,12 +372,13 @@ spdy_on_data_chunk_recv_callback(spdylay_session * /*session*/, uint8_t /*flags*
>                                  size_t len, void *user_data)
> {
>   SpdyClientSession *sm = (SpdyClientSession *)user_data;
> -  SpdyRequest *req = sm->req_map[stream_id];
> +  SpdyRequest *req = NULL;
> +  map<int32_t, SpdyRequest*>::iterator iter = sm->req_map.find(stream_id);
> 
>   //
>   // SpdyRequest has been deleted on error, drop this data;
>   //
> -  if (!req)
> +  if ((iter == sm->req_map.end()) || ((req=iter->second) == NULL))
>     return;
> 
>   Debug("spdy", "++++Fetcher Append Data, len:%zu", len);
> @@ -391,7 +392,8 @@ spdy_on_data_recv_callback(spdylay_session *session, uint8_t flags,
>                            int32_t stream_id, int32_t length, void *user_data)
> {
>   SpdyClientSession *sm = (SpdyClientSession *)user_data;
> -  SpdyRequest *req = sm->req_map[stream_id];
> +  SpdyRequest *req = NULL;
> +  map<int32_t, SpdyRequest*>::iterator iter = sm->req_map.find(stream_id);
> 
>   spdy_show_data_frame("++++RECV", session, flags, stream_id, length, user_data);
> 
> @@ -400,7 +402,7 @@ spdy_on_data_recv_callback(spdylay_session *session, uint8_t flags,
>   // client might continue to send POST data, We should reenable
>   // sm->write_vio so that WINDOW_UPDATE has a chance to be sent.
>   //
> -  if (!req) {
> +  if ((iter == sm->req_map.end()) || ((req=iter->second) == NULL)) {
>     TSVIOReenable(sm->write_vio);
>     return;
>   }
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c9d44335/proxy/spdy/SpdyClientSession.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/spdy/SpdyClientSession.cc b/proxy/spdy/SpdyClientSession.cc
> index 6367e67..f816ba9 100644
> --- a/proxy/spdy/SpdyClientSession.cc
> +++ b/proxy/spdy/SpdyClientSession.cc
> @@ -378,11 +378,12 @@ spdy_read_fetch_body_callback(spdylay_session * /*session*/, int32_t stream_id,
> 
>   SpdyClientSession *sm = (SpdyClientSession *)user_data;
>   SpdyRequest *req = (SpdyRequest *)source->ptr;
> +  map<int32_t, SpdyRequest*>::iterator iter = sm->req_map.find(stream_id);
> 
>   //
>   // req has been deleted, ignore this data.
>   //
> -  if (req != sm->req_map[stream_id]) {
> +  if ((iter == sm->req_map.end()) || (req != iter->second)) {
>     Debug("spdy", "    stream_id:%d, call:%d, req has been deleted, return 0",
>           stream_id, g_call_cnt);
>     *eof = 1;
> 


Re: git commit: TS-2780: Core dump in SpdyRequest::clear() in production testing of SPDY

Posted by Sudheer Vinukonda <su...@yahoo-inc.com.INVALID>.
Good suggestion - will update and resubmit the patch.

Thank you,

Sudheer

On 7/8/14, 12:38 PM, "James Peach" <jp...@apache.org> wrote:

>A helper function would make this cleaner:
>
>SpdyRequest *
>SpdyClientSession::find_request(int streamid) {
>  map<int32_t, SpdyRequest*>::iterator iter =
>this->req_map.find(streamid);
>  return iter == this->req_map.end() ? NULL : iter->second;
>}
>
>On Jul 8, 2014, at 12:29 PM, bcall@apache.org wrote:
>
>> Repository: trafficserver
>> Updated Branches:
>>  refs/heads/master 0c18f7bcd -> c9d443353
>> 
>> 
>> TS-2780: Core dump in SpdyRequest::clear() in production testing of SPDY
>> 
>> 
>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>> Commit: 
>>http://git-wip-us.apache.org/repos/asf/trafficserver/commit/c9d44335
>> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/c9d44335
>> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/c9d44335
>> 
>> Branch: refs/heads/master
>> Commit: c9d4433531767c9b5f6db42b488af4552bc5c4a9
>> Parents: 0c18f7b
>> Author: Sudheer Vinukonda <su...@yahoo-inc.com>
>> Authored: Tue Jul 8 12:28:57 2014 -0700
>> Committer: Bryan Call <bc...@apache.org>
>> Committed: Tue Jul 8 12:28:57 2014 -0700
>> 
>> ----------------------------------------------------------------------
>> proxy/spdy/SpdyCallbacks.cc     | 10 ++++++----
>> proxy/spdy/SpdyClientSession.cc |  3 ++-
>> 2 files changed, 8 insertions(+), 5 deletions(-)
>> ----------------------------------------------------------------------
>> 
>> 
>> 
>>http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c9d44335/proxy/
>>spdy/SpdyCallbacks.cc
>> ----------------------------------------------------------------------
>> diff --git a/proxy/spdy/SpdyCallbacks.cc b/proxy/spdy/SpdyCallbacks.cc
>> index 5a0564f..830c336 100644
>> --- a/proxy/spdy/SpdyCallbacks.cc
>> +++ b/proxy/spdy/SpdyCallbacks.cc
>> @@ -372,12 +372,13 @@ spdy_on_data_chunk_recv_callback(spdylay_session
>>* /*session*/, uint8_t /*flags*
>>                                  size_t len, void *user_data)
>> {
>>   SpdyClientSession *sm = (SpdyClientSession *)user_data;
>> -  SpdyRequest *req = sm->req_map[stream_id];
>> +  SpdyRequest *req = NULL;
>> +  map<int32_t, SpdyRequest*>::iterator iter =
>>sm->req_map.find(stream_id);
>> 
>>   //
>>   // SpdyRequest has been deleted on error, drop this data;
>>   //
>> -  if (!req)
>> +  if ((iter == sm->req_map.end()) || ((req=iter->second) == NULL))
>>     return;
>> 
>>   Debug("spdy", "++++Fetcher Append Data, len:%zu", len);
>> @@ -391,7 +392,8 @@ spdy_on_data_recv_callback(spdylay_session
>>*session, uint8_t flags,
>>                            int32_t stream_id, int32_t length, void
>>*user_data)
>> {
>>   SpdyClientSession *sm = (SpdyClientSession *)user_data;
>> -  SpdyRequest *req = sm->req_map[stream_id];
>> +  SpdyRequest *req = NULL;
>> +  map<int32_t, SpdyRequest*>::iterator iter =
>>sm->req_map.find(stream_id);
>> 
>>   spdy_show_data_frame("++++RECV", session, flags, stream_id, length,
>>user_data);
>> 
>> @@ -400,7 +402,7 @@ spdy_on_data_recv_callback(spdylay_session
>>*session, uint8_t flags,
>>   // client might continue to send POST data, We should reenable
>>   // sm->write_vio so that WINDOW_UPDATE has a chance to be sent.
>>   //
>> -  if (!req) {
>> +  if ((iter == sm->req_map.end()) || ((req=iter->second) == NULL)) {
>>     TSVIOReenable(sm->write_vio);
>>     return;
>>   }
>> 
>> 
>>http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c9d44335/proxy/
>>spdy/SpdyClientSession.cc
>> ----------------------------------------------------------------------
>> diff --git a/proxy/spdy/SpdyClientSession.cc
>>b/proxy/spdy/SpdyClientSession.cc
>> index 6367e67..f816ba9 100644
>> --- a/proxy/spdy/SpdyClientSession.cc
>> +++ b/proxy/spdy/SpdyClientSession.cc
>> @@ -378,11 +378,12 @@ spdy_read_fetch_body_callback(spdylay_session *
>>/*session*/, int32_t stream_id,
>> 
>>   SpdyClientSession *sm = (SpdyClientSession *)user_data;
>>   SpdyRequest *req = (SpdyRequest *)source->ptr;
>> +  map<int32_t, SpdyRequest*>::iterator iter =
>>sm->req_map.find(stream_id);
>> 
>>   //
>>   // req has been deleted, ignore this data.
>>   //
>> -  if (req != sm->req_map[stream_id]) {
>> +  if ((iter == sm->req_map.end()) || (req != iter->second)) {
>>     Debug("spdy", "    stream_id:%d, call:%d, req has been deleted,
>>return 0",
>>           stream_id, g_call_cnt);
>>     *eof = 1;
>> 
>