Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Just use raw close() for discard #70

Closed
wants to merge 1 commit into from
Closed

Just use raw close() for discard #70

wants to merge 1 commit into from

Conversation

jhawthorn
Copy link
Member

Follow up to #65 cc @casperisfine

Previously we were doing a dance with file descriptors in order to replace the connection's fd with one open to /dev/null. Instead we should be able to just close() the fd so long as we don't perform any writes on it.

Previously we were doing a dance with file descriptors in order to
replace the connection's fd with one open to /dev/null. Instead we
should be able to just close() the fd so long as we don't perform any
writes on it.
@casperisfine
Copy link
Contributor

so long as we don't perform any writes on it.

But that's actually the point. To make sure we won't flush any buffers etc.

Is this dance causing issues?

@jhawthorn
Copy link
Member Author

In practice most of the time this probably works fine, but it adds some dangers and unnecessary complexity. open could fail because of fd exhaustion. Calling SSL_shutdown may push errors (which we should be cleaning up anyways). It all should work, I'd just like something similar and not to have an extra state to consider connections could be in (being connected to /dev/null)

I've opened #111 which simplifies this even further and combines the shutdown and discard callbacks.

@jhawthorn jhawthorn closed this Aug 4, 2023
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