Migrate server networking code to Asio 1.36.0 #619
Merged
Conversation
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
"Client" sockets as in the VersionManager acts as a client to connect to something else. Stripping these as they're unused and reduce the overall code to migrate.
twostars
force-pushed
the
asio
branch
4 times, most recently
from
7caee1a to
f7bf3b2
Compare
This removes a bunch of duplicate logic and ensures it's consistently guarded. All log entries are still handled, just externally of the mutex.
It can just allocate these on-demand through asio internals; it's fairly cheap. The initial dummy socket allocs under CIOCPSocket2 are also cheap and don't affect much, so it's not really worth the extra effort managing this.
For efficiency, we keep things like the Send() implementation implemented directly in the implementing class. They tend to handle these differently, so we shouldn't try to one-size-fits-all them, even though most are similar. We also hide away direct access to SocketManager's (formerly CIOCPort) members, so we can more consistently verify them. To avoid any awkward deadlocks (particularly with AI connections in Ebenezer), we ensure listbox updates are done only within the window's main thread. We queue them and post a message to trigger their update when it gets a chance. Otherwise it will use SendMessage(), blocking until the main thread processes, which can be particularly bad if the main thread is doing things with locks already, (e.g. already trying to connect to AI).
We decouple this from shared because it's used by the client. The client has no business knowing about any of this, and will likely down the track end up having its own asio implementation. This would be largely incompatible as it's designed for a server. It makes sense, then, to decouple this and have it in its own server-specific lib project. Also: the shared output directory was using the project default. We should just force it to $(RootDir)bin -- i.e. $(BaseOutDir) -- and just ensure we still reference that, even from server projects. I'm not sure how it got missed when the other output paths got updated. Server\network resides in the server's bin dir (Server\bin) as it's server-specific.
twostars
changed the title
New sends will be blocked, as will handling of any new received packet data. The intent here is to just allow for it to send any final packets before it cuts off the client. This isn't really official behaviour at all, but it's nice to ensure. Also just don't hold _sendMutex longer than we really need in TcpSocket::AsyncSend().
kenner2
approved these changes
Sep 30, 2025
Officially this was checked subtly ('< -1').
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.
In order to move towards our (eventual) goal of supporting cross-platform servers, the hard dependency to Windows sockets needs to go.
This PR aims to address that.
The implementation is more or less a 1:1 implementation for how it already behaves (e.g. UDP sockets still block sends), with the only real difference being that TCP socket sends are asynchronous and queued now.
Beyond that, although I've ultimately migrated everything into its own newly named classes, they should still feel fairly familiar.
CIOCPort->SocketManagerCIOCPSocket2->TcpSocket/TcpClientSocket/TcpServerSocket.CIOCPSocket2was essentially split up into the core TCP socket code (for sending etc.), and then we haveTcpClientSocketfor client socket logic (i.e. connecting to a remote server, like Ebenezer'sCAISocketconnecting to the AI server), andTcpServerSocketfor regular server sockets (i.e. clients connecting to our server, like the KO client connecting to the login or the game server -- namedCUserin both).Also added was a very basic Thread implementation (utilising
std::thread), to effectively replace the existing user threads handling, for example, routine region buffer sends or AI server's sender logic.These threads were just named the same as their original thread function. Probably not the best, but preserves the association there for now.
With regard to say, Ebenezer's
SendWorkerThread, it technically could've been adapted a little closer (e.g. posting a notification to them to handle appropriately), but since it's only triggered via a timer every 200ms, it made more sense to just simplify it to a regular thread processing updates every 200ms.There were also some renames, because the server is generally moving towards more consistent, modern naming, dropping the Hungarian notation. There wasn't too much of that, mostly just the
CIOCPSocket2stuff (andCIOCPortinternals).But regarding the
SocketManager(previouslyCIOCPort) changes; it no longer directly exposes any of its members. That means socket lookups have to be done through a wrapper, allowing us to handle things a little more consistently and ensuring the socket IDs are actually checked (as there was many cases they were not, and completely abusable).These changes extend to essentially not relying on MAX_USER for lookups or scans; it will always use the count actually setup by the manager.
I'm generally not too happy that I shoved another wrapper layer in
CEbenezerDlgfor it (in addition to the socket manager wrappers) but I don't particularly like the socket manager being accessed directly either.Probably all the storage for these things should be moved out and more globally accessible, so it doesn't feel like it's awkwardly tied to this one random class that does everything, but that's probably a future change in general, which most likely ties in with reworking the servers after it's decoupled from MFC, etc.
The AI server specifically had some interesting behaviour where everything copied along the single
CIOCPortinstance's address, in some cases not even using it, and just accessing it directly as it always was able to.I had started to simply change these types & rename these, but as it was entirely pointless, I just stripped them; it can always access it directly.
Finally, for AI, Ebenezer and VersionManager (I didn't bother with Aujard since there's not really any locks; this will go as soon as it's reworked for console anyway), to avoid any awkward deadlocks (particularly with AI connections in Ebenezer), I ensure listbox updates are done only within the window's main thread.
These now get queued and a message is posted to the main thread to trigger their update when it gets a chance.
Otherwise, it would use
SendMessage()which blocks until the main thread processes, which can be particularly bad if the main thread is doing things with conflicting locks already, (e.g. it trying to connect to AI).