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

fix nt accept/connect not initializing with SO_UPDATE_*_CONTEXT #1164

Merged
merged 3 commits into from May 17, 2024

Conversation

G4Vi
Copy link
Collaborator

@G4Vi G4Vi commented May 3, 2024

Fixes calling getpeername, getsockname, getsockopt, setsockopt after accept
and getpeername, getsockname, getsockopt, setsockopt, shutdown after connect.

@jart
Copy link
Owner

jart commented May 3, 2024

Is there any reason why this is a work in progress? Could you explain more about what exactly is happening here?

@G4Vi
Copy link
Collaborator Author

G4Vi commented May 3, 2024

This change alone is an improvement and can be merged now if you like!

Before __imp_setsockopt(args->listensock, SOL_SOCKET, kNtSoUpdateAcceptContext, &handle, sizeof(handle)) was never (at least observed) being called as I think nonblocking io is always used to make the AcceptEx call. This change makes it always be called after a successful accept.

This was marked WIP to see if anything can be done about noticing that the __imp_setsockopt call fails with WSAENOTSOCK if f->handle was created before forking.

@G4Vi G4Vi marked this pull request as ready for review May 7, 2024 06:58
@G4Vi
Copy link
Collaborator Author

G4Vi commented May 7, 2024

Further investigation revealed issues with how sockets are passed to child processes #1174 This is ready for final review, just added test and added link to issue.

@G4Vi G4Vi requested a review from jart May 7, 2024 07:00
@G4Vi G4Vi changed the title fix nt accept not initializing with SO_UPDATE_ACCEPT_CONTEXT fix nt accept/connect not initializing with SO_UPDATE_*_CONTEXT May 8, 2024
@G4Vi
Copy link
Collaborator Author

G4Vi commented May 8, 2024

I just added fixes for the same issue with connect.

@jart jart merged commit 624119e into jart:master May 17, 2024
5 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.

None yet

2 participants