You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "bryancall (via GitHub)" <gi...@apache.org> on 2023/01/31 18:10:53 UTC
[GitHub] [trafficserver] bryancall opened a new pull request, #9342: Fixed memory leaks with http/3
bryancall opened a new pull request, #9342:
URL: https://github.com/apache/trafficserver/pull/9342
- Leak with not freeing packets
- Leak in buffers for QPACK
- Leak with the http/3 session
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1412272171
[approve ci]
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1412905845
[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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1411412334
[approve ci]
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9342: Fixed memory leaks with http/3
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1416535865
Seems like changing base branch confuses GitHub. I see changes already on master.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9342: Fixed memory leaks with http/3
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#discussion_r1092359713
##########
iocore/net/QUICNet.cc:
##########
@@ -77,6 +77,7 @@ QUICPollCont::_process_packet(QUICPollEvent *e, NetHandler *nh)
}
// Note: We should free QUICPollEvent here since vc could be freed from other thread.
+ p->free();
Review Comment:
I found #ifdef. This code is just for quiche and no problem.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on a diff in pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on code in PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#discussion_r1092324077
##########
iocore/net/QUICNet.cc:
##########
@@ -77,6 +77,7 @@ QUICPollCont::_process_packet(QUICPollEvent *e, NetHandler *nh)
}
// Note: We should free QUICPollEvent here since vc could be freed from other thread.
+ p->free();
Review Comment:
`quic_conn_recv()` get called right before this, does that process the packet?
`QUICPollCont::_process_packet() > QUICNetVConnection::handle_received_packet() > quiche_conn_recv()`
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1411224077
[approve ci Rocky]
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9342: Fixed memory leaks with http/3
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#discussion_r1092316635
##########
iocore/net/QUICNet.cc:
##########
@@ -77,6 +77,7 @@ QUICPollCont::_process_packet(QUICPollEvent *e, NetHandler *nh)
}
// Note: We should free QUICPollEvent here since vc could be freed from other thread.
+ p->free();
Review Comment:
I don't think we can free the `p` here because QUICNetVC doesn't immediately process packets in `handle_receiv ed_packet()`. It should be freed on QUICNetVC side.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9342: Fixed memory leaks with http/3
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1419997048
[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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall merged pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall merged PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9342: Fixed memory leaks with http/3
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on code in PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#discussion_r1092322682
##########
iocore/net/QUICNet.cc:
##########
@@ -77,6 +77,7 @@ QUICPollCont::_process_packet(QUICPollEvent *e, NetHandler *nh)
}
// Note: We should free QUICPollEvent here since vc could be freed from other thread.
+ p->free();
Review Comment:
Actually the Quiche version may process packets immediately, but it'd break the original version.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1411259151
[approve ci]
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #9342: Fixed memory leaks with http/3
Posted by "maskit (via GitHub)" <gi...@apache.org>.
maskit commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1412986847
Doesn't the second commit leak application instance on tests? What was the problem?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #9342: Fixed memory leaks with http/3
Posted by "bryancall (via GitHub)" <gi...@apache.org>.
bryancall commented on PR #9342:
URL: https://github.com/apache/trafficserver/pull/9342#issuecomment-1411224339
[approve ci Fedora]
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org