Skip to content

Add Poll to Sockets#2396

Open
zerkawei wants to merge 3 commits intobeefytech:masterfrom
zerkawei:socket-poll
Open

Add Poll to Sockets#2396
zerkawei wants to merge 3 commits intobeefytech:masterfrom
zerkawei:socket-poll

Conversation

@zerkawei
Copy link
Contributor

This PR adds poll() support to Sockets given it's recommended over select() for modern applications.

@bfiete
Copy link
Collaborator

bfiete commented Feb 16, 2026

It looks like this won't work on Windows, as WSAPoll is not called by Poll, only the Linux poll is...

@bfiete
Copy link
Collaborator

bfiete commented Feb 16, 2026

Also you could just name WSAPoll as poll and then use LinkName("WSAPoll") instead of CLink

@zerkawei zerkawei marked this pull request as draft February 18, 2026 12:37
@zerkawei zerkawei marked this pull request as ready for review February 18, 2026 12:51
@zerkawei
Copy link
Contributor Author

I've added shutdown before close to prevent polls from being blocked by a closed socket

@bfiete
Copy link
Collaborator

bfiete commented Feb 18, 2026

Given that Socket is a "thin" API wrapper, I think it's probably better to split out the Close and Shutdown into separate methods.

I believe the destructor should only Close, and if Shutdown behavior is desired then the user should do that himself.

@zerkawei
Copy link
Contributor Author

zerkawei commented Feb 18, 2026

How should Shutdown interact with IsConnected ?
Do we keep track of which side of the connection has been shut down ?
Do we only update IsConnected with Close ?
Do we only permit shut down of both sides at once ?

@bfiete
Copy link
Collaborator

bfiete commented Feb 18, 2026

I think we should permit shutdown with directional flags, just like the actual C shutdown. I don't think it's necessary to keep track of which side is shutdown internally, however, and I think we can consider any shutdown to cause IsConnected to return false.

For our purposes we would define "conneted" to mean "able to send and receive".

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