netplay/tcp: generalize checkSockets() function to be usable with various polling APIs
#4196
Conversation
This file contains 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
The new naming scheme clearly conveys the purpose of the function. Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
ManManson
force-pushed
the
netsocket_lowlevel_polling_generalization
branch
3 times, most recently
from
89d39e3 to
2d40815
Compare
past-due
reviewed
Feb 8, 2025
ManManson
force-pushed
the
netsocket_lowlevel_polling_generalization
branch
from
2d40815 to
c13b40c
Compare
ManManson
force-pushed
the
netsocket_lowlevel_polling_generalization
branch
from
c13b40c to
06d6a8e
Compare
past-due
reviewed
Feb 9, 2025
| // `MAX_TMP_SOCKETS` for `tmp_socket_set` (also in netplay.cpp) | ||
| static constexpr size_t MAX_FDS_COUNT = MAX_CONNECTED_PLAYERS + MAX_TMP_SOCKETS; | ||
|
|
||
| std::array<pollfd, MAX_FDS_COUNT> fds_; |
There was a problem hiding this comment.
With the caveat that, while this is okay as a step 1 for now (because this is only used by checkSocketsReadable), future refactoring to use PollDescriptorSet in socketThreadFunction will require this to not have a hard-coded max limit (and presumably use a std::vector). (As-is, socketThreadFunction could already exceed this limit by at least 1 due to writes to rs_socket.)
There was a problem hiding this comment.
Agreed, will address it in the next PR.
ManManson
force-pushed
the
netsocket_lowlevel_polling_generalization
branch
3 times, most recently
from
567033d to
29ade85
Compare
…arious polling APIs The patch introduces the following abstractions to make it easier to switch between various polling APIs in the TCP_DIRECT netcode: 1. `IDescriptorSet` interface to abstract away the details of a particular polling API (`select()` and `poll()` are supported). 2. `PollEventType` enumeration to be used in concrete subclasses of `IDescriptorSet` to describe the type of events we are interested in, when polling a given descriptor set (generally speaking, both `select` and `poll` can listen for multiple types of events simultaneously, but in our particular case we listen for only one of them at a time). 3. `SelectDescriptorSet<PollEventType>` and `PollDescriptorSet<PollEventType>` descriptor set types which actually implement the support for `select` and `poll` APIs. 4. `checkSocketsReadable()` function now makes direct use of `SelectDescriptorSet`(Windows, note on support below) and `PollDescriptorSet`(Linux/macOS/*BSD). NOTE: We don't use `poll()`(`WSAPoll()`, to be accurate) on Windows for the time being, since it's affected by a bug in Windows versions prior to Windows 10 version 2004 (and there wasn't any prior analysis on how many potential players will be affected): `WSAPoll()` function can time out on socket connection errors instead of returning an error early. For more information on the bug, see: https://stackoverflow.com/questions/21653003/is-this-wsapoll-bug-for-non-blocking-sockets-fixed and also https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsapoll#remarks Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
ManManson
force-pushed
the
netsocket_lowlevel_polling_generalization
branch
from
29ade85 to
0bf7d26
Compare
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.
The patchset introduces the following abstractions to make it easier to switch between various polling APIs in the TCP_DIRECT netcode:
IDescriptorSetinterface to abstract away the details of a particular polling API (select()andpoll()are supported).PollEventTypeenumeration to be used in concrete subclasses ofIDescriptorSetto describe the type of events we are interested in, when polling a given descriptor set (generally speaking, bothselectandpollcan listen for multiple types of events simultaneously, but in our particular case we listen for only one of them at a time).SelectDescriptorSet<PollEventType>andPollDescriptorSet<PollEventType>descriptor set types which actually implement the support forselectandpollAPIs.checkSocketsReadable()function now makes direct use ofSelectDescriptorSet(Windows, note on support below) andPollDescriptorSet(Linux/macOS/*BSD).NOTE: We don't use
poll()(WSAPoll(), to be accurate) on Windows for the time being, since it's affected by a bug in Windows versions prior to Windows 10 version 2004 (and there wasn't any prior analysis on how many potential players will be affected):WSAPoll()function can time out on socket connection errors instead of returning an error early.For more information on the bug, see: https://stackoverflow.com/questions/21653003/is-this-wsapoll-bug-for-non-blocking-sockets-fixed
and also https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-wsapoll#remarks
Signed-off-by: Pavel Solodovnikov pavel.al.solodovnikov@gmail.com