[ 
https://issues.apache.org/jira/browse/TS-2584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13963107#comment-13963107
 ] 

ASF GitHub Bot commented on TS-2584:
------------------------------------

Github user SolidWallOfCode commented on the pull request:

    https://github.com/apache/trafficserver/pull/44#issuecomment-39865474
  
    It seems to me there are two effecrive changes for this. The first minor 
one is that the response can be set earlier in the method. The more concerning 
one is that whether the response is set at all for negative caching. If the 
info is valid on entry then it won't be, but if the entry isn't set then it 
will be. Presumably the difference is that the transform is never valid on 
entry. The big question is whether setting the response can be a problem. 


> failed assert transforming and caching negative response
> --------------------------------------------------------
>
>                 Key: TS-2584
>                 URL: https://issues.apache.org/jira/browse/TS-2584
>             Project: Traffic Server
>          Issue Type: Bug
>          Components: HTTP
>            Reporter: Jack Bates
>              Labels: Review
>             Fix For: 5.0.0
>
>
> If negative caching is enabled and you transform a negative response (for 
> example a null transform) there is a failed assert in 
> HttpTransact::set_headers_for_cache_write()
> {noformat}
> FATAL: HttpTransact.cc:4811: failed assert 
> `cache_info->response_get()->valid()`
> proxy/.libs/lt-traffic_server - STACK TRACE:
> lib/ts/.libs/libtsutil.so.5(ink_fatal_die+0x0)[0xb77a6445]
> lib/ts/.libs/libtsutil.so.5(+0x22269)[0xb77a5269]
> proxy/.libs/lt-traffic_server(_ZN12HttpTransact27set_headers_for_cache_writeEPNS_5StateEP8HTTPInfoP7HTTPHdrS5_+0x1da)[0x81c0ae6]
> proxy/.libs/lt-traffic_server(_ZN12HttpTransact22handle_transform_readyEPNS_5StateE+0x272)[0x81c0576]
> proxy/.libs/lt-traffic_server(_ZN6HttpSM32call_transact_and_set_next_stateEPFvPN12HttpTransact5StateEE+0x78)[0x81a4b3e]
> proxy/.libs/lt-traffic_server(_ZN6HttpSM38state_response_wait_for_transform_readEiPv+0x196)[0x81926b0]
> proxy/.libs/lt-traffic_server(_ZN6HttpSM12main_handlerEiPv+0x2df)[0x81969b5]
> proxy/.libs/lt-traffic_server(_ZN12Continuation11handleEventEiPv+0x47)[0x811b427]
> proxy/.libs/lt-traffic_server(_ZN17TransformTerminus12handle_eventEiPv+0x27f)[0x815d81b]
> proxy/.libs/lt-traffic_server(_ZN12Continuation11handleEventEiPv+0x47)[0x811b427]
> proxy/.libs/lt-traffic_server(_ZN7EThread13process_eventEP5Eventi+0x104)[0x8300692]
> proxy/.libs/lt-traffic_server(_ZN7EThread7executeEv+0xd6)[0x8300916]
> proxy/.libs/lt-traffic_server[0x82ff569]
> /lib/i686/cmov/libpthread.so.0(+0x5955)[0xb7467955]
> /lib/i686/cmov/libc.so.6(clone+0x5e)[0xb717f1de]
> Aborted (core dumped)
> {noformat}
> HttpTransact::handle_transform_ready() passes s->cache_info.transform_store 
> to HttpTransact::set_headers_for_cache_write()
> The ->valid() test at the top of HttpTransact::set_headers_for_cache_write() 
> fails so ->create() gets called.
> {code}
>   if (!cache_info->valid()) {
>     cache_info->create();
>   }
> {code}
> (s->cache_info.transform_store->m_alt is NULL. I didn't look into why this 
> is, is it expected?)
> Because s->negative_caching is true, cache_info->response_set(response) 
> doesn't get called, instead the assert fails.
> {code}
>   if (!s->negative_caching)
>     cache_info->response_set(response);
>   else {
>     ink_assert(cache_info->response_get()->valid());
>   }
> {code}
> Assuming the cache_info->valid() test can fail and s->negative_caching can be 
> true at the same time, one possible solution is to fix the logic so 
> cache_info->response_set(response) gets called?



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to