Listener: fix a crash when updating any listener that does not bind to port#18421
Listener: fix a crash when updating any listener that does not bind to port#18421mattklein123 merged 17 commits intoenvoyproxy:mainfrom
Conversation
lambdai
changed the title
|
@lizan since this is blocking istio-proxy from picking up the change. |
|
ref: istio/istio#34188 |
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks generally LGTM with small comments.
/wait
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
|
/retest |
|
Retrying Azure Pipelines: |
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks, one question.
/wait-any
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
|
LGTM but unfortunately needs another main merge. /wait |
Thanks! Just did it |
|
@mattklein123 Thank you for the fast and friendly collaboration! |
|
I see conflict show up again at WebUI but my origin/main is updated. I will keep watch on this |
|
@lambdai do we want to backport this to 1.20.x? |
yes, I plan to propose /backport when this is merged. |
|
Should I do it earlier? |
|
/backport |
repokitteh-read-only
bot
added
the
backport/review
|
@lizan @mathetake This is 1.20 only and it's critical for istio release See istio/istio#34188 I can create the PR when this one is merged. WDTY? |
…o port (envoyproxy#18421) Signed-off-by: Yuchen Dai <silentdai@gmail.com>
|
@mathetake is OOO Yes we should backport this to 1.20.x |
…o port (#18421) (#18505) backport e84987c to v1.20 Fix the crash caused by listener update when the any listener set bind_to_port to false. A listener does not bind to port has no io handle. close() already check it. Now add the check to isOpen(). Also the Tcp(or Udp)ListenSocket::duplicate() creates parent ListenSocketImpl Make it returning the derived ListenSocket, either TcpListenSocket or UdpListenSocket Signed-off-by: Yuchen Dai <silentdai@gmail.com>
alyssawilk
added
backport/approved
…o port (envoyproxy#18421) (envoyproxy#18505) backport e84987c to v1.20 Fix the crash caused by listener update when the any listener set bind_to_port to false. A listener does not bind to port has no io handle. close() already check it. Now add the check to isOpen(). Also the Tcp(or Udp)ListenSocket::duplicate() creates parent ListenSocketImpl Make it returning the derived ListenSocket, either TcpListenSocket or UdpListenSocket Signed-off-by: Yuchen Dai <silentdai@gmail.com>
6 participants
Commit Message:
Fix the crash caused by listener update when the any listener set bind_to_port to false.
A listener does not bind to port has no io handle. close() already check it.
Now add the check to isOpen().
Also the Tcp(or Udp)ListenSocket::duplicate() creates parent ListenSocketImpl
Make it returning the derived ListenSocket, either TcpListenSocket or UdpListenSocket
Additional Description:
Risk Level:
Testing: Unit test on duplicate. Integration test on bind_to_port.
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]