Skip to content

Avoid 10 minute goroutine leak in error case for handled errors#99

Merged
dims merged 1 commit into
moby:masterfrom
liggitt:closeleak
Jun 27, 2024
Merged

Avoid 10 minute goroutine leak in error case for handled errors#99
dims merged 1 commit into
moby:masterfrom
liggitt:closeleak

Conversation

@liggitt

@liggitt liggitt commented Jun 27, 2024

Copy link
Copy Markdown
Contributor

Addresses #92 for handled errors

I think there's more we could do here to shorten the 10 minute hang for unhandled errors, but this is a smaller safer first improvement.

cc @aojea @seans3

Signed-off-by: Jordan Liggitt <liggitt@google.com>
@thaJeztah

Copy link
Copy Markdown
Member

@dmcgowan @dims ptal

@thaJeztah

Copy link
Copy Markdown
Member

FWIW; I'm also happy to add more maintainers from the k8s folks familiar with the code and how it's used in k8s if that helps; I think k8s is currently the only consumer of this module (leaving that to @dims and others to consider; just wanted to mention that I wouldn't have objections)

Comment thread connection.go
@aojea

aojea commented Jun 27, 2024

Copy link
Copy Markdown

we are consuming it and it seems the plan we had to deprecate is going to take some time, we are interested in keeping it solid and only to fix bugs

It should not be problematic, at least from my side, as it seems it has been running in production for lot of years

@aojea

aojea commented Jun 27, 2024

Copy link
Copy Markdown

LGTM

@dims dims left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

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.

4 participants