Skip to content

Conversation

@dmitryskorbovenko
Copy link
Contributor

Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! 😃

Does this definitely fix the issues you were seeing in mitmproxy/mitmproxy#7675? Or is this a speculative fix that still needs testing?

@dmitryskorbovenko
Copy link
Contributor Author

Thank you for the PR! 😃

Does this definitely fix the issues you were seeing in mitmproxy/mitmproxy#7675? Or is this a speculative fix that still needs testing?

I use this in my app ui tests run every day, haven't observed "Too many open files" issue any more.

@dmitryskorbovenko dmitryskorbovenko force-pushed the issue/7675-connection-leak branch from 70c626c to 476428a Compare May 28, 2025 09:46
Copy link
Member

@mhils mhils left a comment

Choose a reason for hiding this comment

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

Thank you! For posteriority, I believe this PR may break implementations that rely on TCP half-close. But practically speaking that is less important than the cleanup issues you described, so let's go ahead with this now and revisit if necessary.

@mhils mhils enabled auto-merge (squash) May 29, 2025 13:21
@mhils mhils merged commit 2905bbb into mitmproxy:main May 29, 2025
23 checks passed
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.

2 participants