refactor: improve multi_socket_cb implementation#260
Merged
Conversation
Member
|
@imor Can we prefix this PR as
|
Contributor
Author
|
@steve-chavez yeah I intend to clean this PR up. I added this as a draft so that I don't forget. I only needed this change to run the tests on my local setup. |
steve-chavez
approved these changes
Jul 7, 2026
steve-chavez
left a comment
Member
There was a problem hiding this comment.
@imor Looks good! On second thought I'll change this to a fix: commit since it also changes the logic for epoll. That way the change clearly shows on the release (in case if something goes wrong).
Member
|
A further improvement could be to run the loadtest on macos, that should prove pg_net works the same there pg_net/.github/workflows/main.yml Lines 56 to 78 in 0ab4bae |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR improves the
multi_socket_cbimplementation for both kqueue and epoll by doing the following:For kqueue:
sock_info->actionwas not being updated. This also lead to a use-after-free bug and sometimes made the tests on macOS fail due to a segfault. All these bugs are fixed.curl_multi_assignand raises a Postgres error if the call fails.For epoll:
curl_multi_assignand raises a Postgres error if the call fails.what == CURL_POLL_INOUTby setting the rightEPOLLINandEPOLLOUTcombination inev.eventsmember.curl_multi_assignthe first timemulti_socket_cbis called for a socket fd and removes it whenwhat == CURL_POLL_REMOVE. This clarifies the intent of thecurl_multi_assigncall because the older code assignments tosocket_existsvariable were never read and were confusing.