During the process of attempting to revive the Haiku port, I've been forced to delve deep into SRB2's netcode due to various issues arising when trying to connect to a server. In the process of this, a lot of issues in SRB2's netcode has come to my attention, and seem to break horribly on Haiku.
One of these is how select() is used for checking if a socket is available for use. In theory, this is sensible: checking before using a socket provides a safe way to implement a fallback mechanism in case sending or receiving data is not possible at the time. The problem, however, is in how this was implemented, and there are a lot of thing that are wrong in the implementation.
First, let's start by addressing the obvious: select() is obsolete and superseded by poll(). Why? Well, quoting the manpages for select():
WARNING: select() can monitor only file descriptors numbers that are less than FD_SETSIZE (1024)—an unreasonably low limit for many modern applications—and this limitation will not change. All modern applications should instead use poll(2) or epoll(7), which do not suffer this limitation.
Now, I want to emphasize on a thing here: it says file descriptors numbers, not amount of file descriptors. That is, the index that a file descriptor is assigned to. Since this is outside of the application's control (and, on some OSes, even globally shared), these can easily exceed that limit, and this is actually something I've observed in the wild, but only now have I been able to figure out the cause. So, logically, just replace it with poll() and we'd be golden, right?
Wrong. There are more issues with this.
Let me explain by showing you the select() line from the code itself:
rselect = select(255, &tset, NULL, NULL, &timeval_for_select);Notice something wrong? Don't worry, I don't blame you if you don't, but let me give you a hint: select() works with file descriptor numbers, not the amount of file descriptors. To explain what I mean with that, let's turn ourselves towards the manpage:
nfds This argument should be set to the highest-numbered file descriptor in any of the three sets, plus 1. The indicated file descriptors in each set are checked, up to this limit (but see BUGS).
nfds here is the first argument, where it unconditionally passes 255. If any of the sockets are above that, then (on some OSes) the file descriptor will be ignored. From my experimentation, this is why the game occasionally softlocks once they join a server, as it causes some file descriptors to be permanently blocked upon joining a server. However, this all sounds like it can be fixed alongside replacing it with poll(), right?
No. There is more.
Quoting the manpage once more:
select() allows a program to monitor multiple file descriptors, waiting until one or more of the file descriptors become "ready" for some class of I/O operation (e.g., input possible).
Let me emphasize that it says one or more. Why is this a problem? Simple: we're passing all sockets to it, and SRB2 operates with more than one. Specifically, SRB2 works by creating two sockets: one for IPv4 and one for IPv6. Since IPv6 has been unusable up until recently with the IPv6 support that has been added to 2.2.14, this socket has always been available for use, meaning that the IPv4 socket has been sending and receiving data even in cases where it has been unable to. This has a big implication: this entire logic has had no purpose whatsoever. It's purely been a source of bugs and it has failed to serve the only purpose that it was meant to serve.
This reason is the key to why we're just better of removing it instead of trying to fix it, since just changing it to poll() won't solve this problem. And since it has never worked to begin with, there really is no point of trying to fix it either, since evidently, SRB2 runs and has ran just fine for years without it. So, let's just do the easy thing and remove it, and not make a big deal out of it.