Nat type detection involves 4 extra ports on the server and 1 extra port on the client. This bug uses the naming conventions from NatTypeDetectionServer.h (S1P2, S2P3, S3P4, S4P5, C2) to help describe the issue.
C2 and S3P4 are used for listening only and are created as non-blocking sockets (bbp.nonBlockingSocket=true in RakNet::CreateNonblockingBoundSocket). They would be better off as blocking sockets so that RNS2_Berkley::RecvFromLoop doesn't consume a large amount of CPU as it spins with only a RakSleep(0); to yield time.
S1P2, S2P3 and S4P5 are used for UDP sends only, but RakNet::CreateNonblockingBoundSocket calls CreateRecvPollingThread to create an unnecessary recv thread (which also spins on a RakSleep(0) as they're non-blocking sockets).
C2 and S3P4 end up with two recv threads because RakNet::CreateNonblockingBoundSocket calls CreateRecvPollingThread, as does NatTypeDetectionClient::DetectNATType and NatTypeDetectionServer::Startup respectively.
In all the server has 5 threads spinning on a RakSleep(0), none of which are necessary.
Suggested fix:
C2 and S3P4 should be created as blocking sockets
The redundant call to CreateRecvPollingThread in RakNet::CreateNonblockingBoundSocket should be removed.
Remarks
The extra CPU usage here is pretty harmless for the client because the C2 socket is so short-lived. It's also relatively harmless for the server if you're running on a dedicated machine as the spinning threads will just be consuming spare cycles (and wasting power). However, if you're running the detection server on a system where you pay extra for CPU utilization (e.g. AWS) then it is worth fixing.
Hakunametata
commented
6 years ago
Description
Nat type detection involves 4 extra ports on the server and 1 extra port on the client. This bug uses the naming conventions from NatTypeDetectionServer.h (S1P2, S2P3, S3P4, S4P5, C2) to help describe the issue.
C2 and S3P4 are used for listening only and are created as non-blocking sockets (bbp.nonBlockingSocket=true in RakNet::CreateNonblockingBoundSocket). They would be better off as blocking sockets so that RNS2_Berkley::RecvFromLoop doesn't consume a large amount of CPU as it spins with only a RakSleep(0); to yield time.
S1P2, S2P3 and S4P5 are used for UDP sends only, but RakNet::CreateNonblockingBoundSocket calls CreateRecvPollingThread to create an unnecessary recv thread (which also spins on a RakSleep(0) as they're non-blocking sockets).
C2 and S3P4 end up with two recv threads because RakNet::CreateNonblockingBoundSocket calls CreateRecvPollingThread, as does NatTypeDetectionClient::DetectNATType and NatTypeDetectionServer::Startup respectively.
In all the server has 5 threads spinning on a RakSleep(0), none of which are necessary.
Suggested fix:
Remarks
The extra CPU usage here is pretty harmless for the client because the C2 socket is so short-lived. It's also relatively harmless for the server if you're running on a dedicated machine as the spinning threads will just be consuming spare cycles (and wasting power). However, if you're running the detection server on a system where you pay extra for CPU utilization (e.g. AWS) then it is worth fixing.