Skip to content

conn pool: fix crash introduced in #16948#17302

Merged
ggreenway merged 1 commit intoenvoyproxy:mainfrom
ggreenway:fix-pool-crash
Jul 13, 2021
Merged

conn pool: fix crash introduced in #16948#17302
ggreenway merged 1 commit intoenvoyproxy:mainfrom
ggreenway:fix-pool-crash

Conversation

@ggreenway
Copy link
Copy Markdown
Member

Signed-off-by: Greg Greenway ggreenway@apple.com

Commit Message: If a host is transitioned to unhealthy, and closing its idle connections results in the pool becoming idle, it would trigger this crash.
Additional Description:
Risk Level: Low
Testing: Added new test; verified that the test without the fix crashes in ASAN.
Docs Changes: none
Release Notes: none
Platform Specific Features: none
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

If a host is transitioned to unhealthy, and closing its idle
connections results in the pool becoming idle, it would trigger this crash.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
Copy link
Copy Markdown
Member

@rgs1 rgs1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified fix in one of our canaries were this originally triggered, thanks!

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@ggreenway
Copy link
Copy Markdown
Member Author

@rgs1 Thanks a lot for your help in figuring out the crash!

@ggreenway ggreenway merged commit 3c266bb into envoyproxy:main Jul 13, 2021
ggreenway added a commit to ggreenway/envoy that referenced this pull request Jul 13, 2021
…xy#17302)"

This reverts commit 3c266bb.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
ggreenway added a commit to ggreenway/envoy that referenced this pull request Jul 19, 2021
Delete connection pools when they have no connections anymore. This
fixes unbounded memory use for cases where a new connection pool is
needed for each downstream connection, such as when using upstream
PROXY protocol.

Fixes envoyproxy#16682

This reverts commit b7bc539.
This reverts PR envoyproxy#17319, by re-adding envoyproxy#17302 and envoyproxy#16948.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
Co-authored-by: Craig Radcliffe <craig.radcliffe@broadcom.com>
ggreenway added a commit that referenced this pull request Jul 26, 2021
Delete connection pools when they have no connections anymore. This
fixes unbounded memory use for cases where a new connection pool is
needed for each downstream connection, such as when using upstream
PROXY protocol.

This reverts commit b7bc539.
This reverts PR #17319, by re-adding #17302 and #16948.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
Co-authored-by: Craig Radcliffe <craig.radcliffe@broadcom.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
If a host is transitioned to unhealthy, and closing its idle
connections results in the pool becoming idle, it would trigger this crash.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
Delete connection pools when they have no connections anymore. This
fixes unbounded memory use for cases where a new connection pool is
needed for each downstream connection, such as when using upstream
PROXY protocol.

This reverts commit b7bc539.
This reverts PR envoyproxy#17319, by re-adding envoyproxy#17302 and envoyproxy#16948.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
Co-authored-by: Craig Radcliffe <craig.radcliffe@broadcom.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants