Github user gtenev commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1100#discussion_r83512075
  
    --- Diff: proxy/http2/Http2Stream.cc ---
    @@ -267,10 +267,12 @@ Http2Stream::do_io_close(int /* flags */)
           // Make sure any trailing end of stream frames are sent
           // Ourselve will be removed at send_data_frames or closing 
connection phase
           static_cast<Http2ClientSession 
*>(parent)->connection_state.send_data_frames(this);
    +
    +      // Make sure the stream is deleted at this point since next step is 
self destroy.
    +      this->delete_stream();
    --- End diff --
    
    This is what actually made sure we delete the stream from the `DLL<>` 
before  triggering `destroy()` (before leaving `do_io_close()`).
    
    In the version 6.2.1 we have `send_data_frames()` delete the stream from 
`DLL<>` on `HTTP2_STREAM_STATE_CLOSED`.
    
    Then in a later version we added `HTTP2_STREAM_STATE_HALF_CLOSED_LOCAL`. 
    
    What caused the destroying of `DLL<>` in our case was 
`HTTP2_STREAM_STATE_HALF_CLOSED_REMOTE` (which does not cause deletion in any 
version).
    
    It seemed to me we have been always vulnerable to this problem despite the 
fixes. That is why I thought I would add this “catch-all-delete-stream” 
line here for all the current and future missing states. After this point we 
are going to `destroy()` the stream  regardless of its state anyway.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to