You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/05/12 06:19:21 UTC
[GitHub] [trafficserver] masaori335 opened a new pull request #7814: Cleanup: Remove a meaningless Http2Stream::do_io_close() call
masaori335 opened a new pull request #7814:
URL: https://github.com/apache/trafficserver/pull/7814
At the begging of `Http2Stream::initiating_close()`, the `closed` flag is set `true`. Calling `Http2Stream::do_io_close()` after setting the flag does nothing.
https://github.com/apache/trafficserver/blob/31a580d0d89b2c141655ea167e4ad1b6b4e4973c/proxy/http2/Http2Stream.cc#L500
https://github.com/apache/trafficserver/blob/31a580d0d89b2c141655ea167e4ad1b6b4e4973c/proxy/http2/Http2Stream.cc#L425
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on pull request #7814: Cleanup: Remove a meaningless Http2Stream::do_io_close() call
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7814:
URL: https://github.com/apache/trafficserver/pull/7814#issuecomment-839777490
Good point about the closed flag and do_io_close.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 removed a comment on pull request #7814: Cleanup: Remove a meaningless Http2Stream::do_io_close() call
Posted by GitBox <gi...@apache.org>.
masaori335 removed a comment on pull request #7814:
URL: https://github.com/apache/trafficserver/pull/7814#issuecomment-839538283
[approve ci autest]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #7814: Cleanup: Remove a meaningless Http2Stream::do_io_close() call
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7814:
URL: https://github.com/apache/trafficserver/pull/7814#issuecomment-839514858
Is `do_io_close` called at somewhere else with `closed` flag off? A right way might be setting `closed` flag `true` after calling `do_io_close`.
Having this pattern below at two places seems very odd -- it implies calling either of those is enough but I think `do_io_close` should be always called if `do_io_read/write()` was called, like `open()` and `close()` system calls.
```
if (closed) {
closed = true;
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #7814: Cleanup: Remove a meaningless Http2Stream::do_io_close() call
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7814:
URL: https://github.com/apache/trafficserver/pull/7814#issuecomment-839538283
[approve ci autest]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on pull request #7814: Cleanup: Remove a meaningless Http2Stream::do_io_close() call
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7814:
URL: https://github.com/apache/trafficserver/pull/7814#issuecomment-839537948
Basically, HttpSM/HttpTunnel should call ProxyTransaction's do_io_close() because it called do_io_read()/write().
For the closed flag, the current code is pretty wired. I'm working on getting rid of these flags and fixing the closing sequence of Http2Stream.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 merged pull request #7814: Cleanup: Remove a meaningless Http2Stream::do_io_close() call
Posted by GitBox <gi...@apache.org>.
masaori335 merged pull request #7814:
URL: https://github.com/apache/trafficserver/pull/7814
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org