diff --git a/Samples/ChatExample/Client/Chat Example Client.cpp b/Samples/ChatExample/Client/Chat Example Client.cpp index 37ae9c165..9c669cd3f 100644 --- a/Samples/ChatExample/Client/Chat Example Client.cpp +++ b/Samples/ChatExample/Client/Chat Example Client.cpp @@ -22,6 +22,7 @@ #include "mafianet/MessageIdentifiers.h" #include "mafianet/peerinterface.h" +#include "mafianet/PeerHandle.h" #include "mafianet/statistics.h" #include "mafianet/types.h" #include "mafianet/BitStream.h" @@ -47,24 +48,19 @@ #include "mafianet/SecureHandshake.h" // Include header for secure handshake #endif -// We copy this from Multiplayer.cpp to keep things all in one file for this example -unsigned char GetPacketIdentifier(MafiaNet::Packet *p); - int main(void) { MafiaNet::RakNetStatistics *rss; // Pointers to the interfaces of our server and client. // Note we can easily have both in the same program - MafiaNet::RakPeerInterface *client= MafiaNet::RakPeerInterface::GetInstance(); + MafiaNet::Peer peer; + MafiaNet::RakPeerInterface *client = peer.get(); // client->InitializeSecurity(0,0,0,0); //MafiaNet::PacketLogger packetLogger; //client->AttachPlugin(&packetLogger); - // Holds packets - MafiaNet::Packet* p; - - // GetPacketIdentifier returns this + // Holds the packet identifier unsigned char packetIdentifier; // Just so we can remember where the packet came from @@ -274,10 +270,10 @@ int main(void) // Get a packet from either the server or the client - for (p=client->Receive(); p; client->DeallocatePacket(p), p=client->Receive()) + for (MafiaNet::PacketPtr p = peer.receive(); p; p = peer.receive()) { - // We got a packet, get the identifier with our handy function - packetIdentifier = GetPacketIdentifier(p); + // We got a packet, get the identifier (ID_TIMESTAMP-aware) + packetIdentifier = p.id(); // Check if this is a network message packet switch (packetIdentifier) @@ -344,25 +340,5 @@ int main(void) // Be nice and let the server know we quit. client->Shutdown(300); - // We're done with the network - MafiaNet::RakPeerInterface::DestroyInstance(client); - return 0; } - -// Copied from Multiplayer.cpp -// If the first byte is ID_TIMESTAMP, then we want the 5th byte -// Otherwise we want the 1st byte -unsigned char GetPacketIdentifier(MafiaNet::Packet *p) -{ - if (p==0) - return 255; - - if ((unsigned char)p->data[0] == ID_TIMESTAMP) - { - RakAssert(p->length > sizeof(MafiaNet::MessageID) + sizeof(MafiaNet::Time)); - return (unsigned char) p->data[sizeof(MafiaNet::MessageID) + sizeof(MafiaNet::Time)]; - } - else - return (unsigned char) p->data[0]; -} diff --git a/Samples/Tests/IncludeAllTests.h b/Samples/Tests/IncludeAllTests.h index 2e1a2648d..53b3179a1 100644 --- a/Samples/Tests/IncludeAllTests.h +++ b/Samples/Tests/IncludeAllTests.h @@ -35,4 +35,5 @@ #include "DisconnectReasonTest.h" #include "PeerGuidTest.h" #include "PointGridSectorizerTest.h" +#include "PeerHandleTest.h" diff --git a/Samples/Tests/PeerHandleTest.cpp b/Samples/Tests/PeerHandleTest.cpp new file mode 100644 index 000000000..9c33cb0eb --- /dev/null +++ b/Samples/Tests/PeerHandleTest.cpp @@ -0,0 +1,230 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + */ + +#include "PeerHandleTest.h" + +#include // std::move + +// A distinct port so a full-suite run does not collide with other tests. +static const unsigned short PEERHANDLE_TEST_PORT = 61015; + +// A user message id carried behind an ID_TIMESTAMP prefix, used to exercise the +// timestamp-aware branch of PacketPtr::id(). +static const unsigned char PEERHANDLE_TS_ID = (unsigned char) (ID_USER_PACKET_ENUM + 7); + +int PeerHandleTest::RunTest(DataStructures::List params, bool isVerbose, bool noPauses) +{ + (void) params; (void) noPauses; + + // --- 1. Peer: movable, non-copyable, moved-from is empty --- + { + Peer a; + if (!a || a.get() == nullptr) + { + if (isVerbose) printf("default-constructed Peer is not valid\n"); + return 1; + } + RakPeerInterface *raw = a.get(); + + Peer b(std::move(a)); + if (a || a.get() != nullptr) + { + if (isVerbose) printf("moved-from Peer (ctor) is not empty\n"); + return 2; + } + if (!b || b.get() != raw) + { + if (isVerbose) printf("move ctor did not transfer the instance\n"); + return 3; + } + + Peer c; + c = std::move(b); + if (b || b.get() != nullptr) + { + if (isVerbose) printf("moved-from Peer (assign) is not empty\n"); + return 4; + } + if (!c || c.get() != raw) + { + if (isVerbose) printf("move assign did not transfer the instance\n"); + return 5; + } + // a and b destruct empty (no-op); c destructs the single live instance. + } + + // --- 2. PacketPtr: null/empty path and empty receive() --- + { + Peer p; + PacketPtr nullPkt(p.get(), nullptr); + if (nullPkt) + { + if (isVerbose) printf("null PacketPtr is unexpectedly truthy\n"); + return 10; + } + if (nullPkt.id() != 255) + { + if (isVerbose) printf("null PacketPtr id() is not 255\n"); + return 11; + } + + // A peer that was never started has nothing queued. + PacketPtr none = p.receive(); + if (none) + { + if (isVerbose) printf("receive() on an un-started peer is not empty\n"); + return 12; + } + } + + // --- 3. Real packets over loopback: plain and timestamp id() branches --- + Peer server; + Peer client; + + SocketDescriptor serverSd(PEERHANDLE_TEST_PORT, 0); + if (server->Startup(1, &serverSd, 1) != RAKNET_STARTED) + { + if (isVerbose) printf("server Startup failed (port %u in use?)\n", PEERHANDLE_TEST_PORT); + return 20; + } + server->SetMaximumIncomingConnections(1); + + SocketDescriptor clientSd; + if (client->Startup(1, &clientSd, 1) != RAKNET_STARTED) + { + if (isVerbose) printf("client Startup failed\n"); + return 21; + } + + SystemAddress serverAddress; + serverAddress.SetBinaryAddress("127.0.0.1"); + serverAddress.SetPortHostOrder(PEERHANDLE_TEST_PORT); + + if (client->Connect("127.0.0.1", PEERHANDLE_TEST_PORT, 0, 0) != CONNECTION_ATTEMPT_STARTED) + { + if (isVerbose) printf("Connect did not start\n"); + return 22; + } + + // Wait for ID_CONNECTION_REQUEST_ACCEPTED on the client. This identifier is + // NOT timestamp-prefixed, so it exercises id()'s plain (first-byte) branch. + bool connected = false; + bool plainBranchOk = false; + TimeMS start = GetTimeMS(); + while (GetTimeMS() - start < 10000 && !connected) + { + for (PacketPtr pkt = server.receive(); pkt; pkt = server.receive()) + { + // Drain the server side (ID_NEW_INCOMING_CONNECTION, etc.). + } + for (PacketPtr pkt = client.receive(); pkt; pkt = client.receive()) + { + if (pkt.id() == ID_CONNECTION_REQUEST_ACCEPTED) + { + if (pkt->data[0] == ID_TIMESTAMP || pkt.id() != (unsigned char) pkt->data[0]) + { + if (isVerbose) printf("plain-branch id() did not match data[0]\n"); + return 23; + } + plainBranchOk = true; + connected = true; + } + } + RakSleep(30); + } + if (!connected) + { + if (isVerbose) printf("client did not connect within 10s\n"); + return 24; + } + if (!plainBranchOk) + return 25; + + // Send a timestamp-prefixed user message and verify id() reads the byte that + // follows the MessageID + Time prefix on the server side. + BitStream bs; + bs.Write((MessageID) ID_TIMESTAMP); + bs.Write(GetTime()); + bs.Write((MessageID) PEERHANDLE_TS_ID); + client->Send(&bs, Priority::High, Reliability::ReliableOrdered, 0, serverAddress, false); + + bool timestampBranchOk = false; + start = GetTimeMS(); + while (GetTimeMS() - start < 5000 && !timestampBranchOk) + { + for (PacketPtr pkt = server.receive(); pkt; pkt = server.receive()) + { + if (pkt->data[0] == ID_TIMESTAMP) + { + if (pkt.id() != PEERHANDLE_TS_ID) + { + if (isVerbose) printf("timestamp-branch id() returned the wrong byte\n"); + return 30; + } + timestampBranchOk = true; + } + } + for (PacketPtr pkt = client.receive(); pkt; pkt = client.receive()) + { + // Drain the client side. + } + RakSleep(30); + } + if (!timestampBranchOk) + { + if (isVerbose) printf("server did not receive the timestamp message within 5s\n"); + return 31; + } + + // server and client are RAII Peers: they destruct (DestroyInstance) here, + // including on every early return above — that is the point of the wrappers. + return 0; +} + +PeerHandleTest::PeerHandleTest(void) +{ +} + +PeerHandleTest::~PeerHandleTest(void) +{ +} + +RakString PeerHandleTest::GetTestName(void) +{ + return "PeerHandleTest"; +} + +RakString PeerHandleTest::ErrorCodeToString(int errorCode) +{ + switch (errorCode) + { + case 0: return "No error"; + case 1: return "Default-constructed Peer invalid"; + case 2: return "Moved-from Peer (ctor) not empty"; + case 3: return "Move ctor did not transfer instance"; + case 4: return "Moved-from Peer (assign) not empty"; + case 5: return "Move assign did not transfer instance"; + case 10: return "Null PacketPtr truthy"; + case 11: return "Null PacketPtr id() != 255"; + case 12: return "receive() on un-started peer not empty"; + case 20: return "Server Startup failed"; + case 21: return "Client Startup failed"; + case 22: return "Connect did not start"; + case 23: return "Plain-branch id() mismatch"; + case 24: return "Client did not connect"; + case 25: return "Did not observe ID_CONNECTION_REQUEST_ACCEPTED"; + case 30: return "Timestamp-branch id() mismatch"; + case 31: return "Server did not receive timestamp message"; + default: return "Undefined error"; + } +} + +void PeerHandleTest::DestroyPeers(void) +{ + // Peers in this test are RAII MafiaNet::Peer locals; they clean themselves + // up at scope exit, so there is nothing to do here. +} diff --git a/Samples/Tests/PeerHandleTest.h b/Samples/Tests/PeerHandleTest.h new file mode 100644 index 000000000..045f815e5 --- /dev/null +++ b/Samples/Tests/PeerHandleTest.h @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2026, MafiaHub + * + * This source code is licensed under the MIT-style license found in the + * license.txt file in the root directory of this source tree. + */ + +#pragma once + + +#include "TestInterface.h" + +#include "mafianet/string.h" +#include "mafianet/DS_List.h" +#include "mafianet/peerinterface.h" +#include "mafianet/PeerHandle.h" +#include "mafianet/MessageIdentifiers.h" +#include "mafianet/BitStream.h" +#include "mafianet/sleep.h" +#include "mafianet/time.h" +#include "mafianet/GetTime.h" + +using namespace MafiaNet; + +/// Verifies the RAII handles Peer / PacketPtr (issue #16): +/// - Peer is movable and non-copyable; a moved-from handle is empty (no +/// double-free at scope exit). +/// - PacketPtr::id() returns 255 for a null/empty packet, the first byte for a +/// plain packet, and the post-timestamp byte for an ID_TIMESTAMP packet. +/// - Peer::receive() yields an empty PacketPtr when nothing is queued, and the +/// move-assignment in a receive loop frees each real packet without leaking. +class PeerHandleTest : public TestInterface +{ +public: + PeerHandleTest(void); + ~PeerHandleTest(void); + int RunTest(DataStructures::List params, bool isVerbose, bool noPauses); + RakString GetTestName(); + RakString ErrorCodeToString(int errorCode); + void DestroyPeers(); +}; diff --git a/Samples/Tests/Tests.cpp b/Samples/Tests/Tests.cpp index 3787f51ed..0576cf3c7 100644 --- a/Samples/Tests/Tests.cpp +++ b/Samples/Tests/Tests.cpp @@ -63,6 +63,7 @@ int main(int argc, char *argv[]) testList.Push(new DisconnectReasonTest(),_FILE_AND_LINE_); testList.Push(new PeerGuidTest(),_FILE_AND_LINE_); testList.Push(new PointGridSectorizerTest(),_FILE_AND_LINE_); + testList.Push(new PeerHandleTest(),_FILE_AND_LINE_); testListSize=testList.Size(); diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index bae5041ed..4d714cdba 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -60,6 +60,7 @@ set(MAFIANET_SOURCES src/PacketizedTCP.cpp src/PacketLogger.cpp src/PacketOutputWindowLogger.cpp + src/PeerHandle.cpp src/PluginInterface2.cpp src/PointGridSectorizer.cpp src/PS4Includes.cpp @@ -217,6 +218,7 @@ set(MAFIANET_HEADERS include/mafianet/PacketOutputWindowLogger.h include/mafianet/PacketPool.h include/mafianet/PacketPriority.h + include/mafianet/PeerHandle.h include/mafianet/peer.h include/mafianet/peerinterface.h include/mafianet/PluginInterface2.h diff --git a/Source/include/mafianet/PeerHandle.h b/Source/include/mafianet/PeerHandle.h new file mode 100644 index 000000000..255235b0e --- /dev/null +++ b/Source/include/mafianet/PeerHandle.h @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2019, SLikeSoft UG (haftungsbeschraenkt) + * + * This source code is licensed under the MIT-style license found in the license.txt + * file in the root directory of this source tree. + */ + +/// \file PeerHandle.h +/// \brief RAII handles for the two core MafiaNet resources. +/// +/// \ref MafiaNet::Peer owns a RakPeerInterface instance (GetInstance/DestroyInstance); +/// \ref MafiaNet::PacketPtr owns a Packet received from it (Receive/DeallocatePacket). +/// Both are movable, non-copyable and exception-safe. This is purely additive — +/// the raw factory/Receive API remains available and unchanged. + +#pragma once + +#include "mafianet/peerinterface.h" // RakPeerInterface, Receive, DeallocatePacket, GetInstance, DestroyInstance +#include "mafianet/types.h" // Packet +#include "mafianet/Export.h" // RAK_DLL_EXPORT + +namespace MafiaNet { + +/// \brief Owning handle for a Packet returned by RakPeerInterface::Receive(). +/// Calls DeallocatePacket() in its destructor. Movable, non-copyable. +class RAK_DLL_EXPORT PacketPtr { +public: + /// Takes ownership of \a p, which must have been produced by \a owner. + PacketPtr(RakPeerInterface* owner, Packet* p) : owner_(owner), p_(p) {} + ~PacketPtr(); + + PacketPtr(PacketPtr&& o) noexcept; + PacketPtr& operator=(PacketPtr&& o) noexcept; + PacketPtr(const PacketPtr&) = delete; + PacketPtr& operator=(const PacketPtr&) = delete; + + Packet* operator->() const { return p_; } + Packet& operator*() const { return *p_; } + Packet* get() const { return p_; } + explicit operator bool() const { return p_ != nullptr; } + + /// Message identifier, accounting for an ID_TIMESTAMP prefix. + /// Returns 255 when the packet is null or empty. + unsigned char id() const; + +private: + RakPeerInterface* owner_; + Packet* p_; +}; + +/// \brief Owning handle for a RakPeerInterface instance. +/// Calls DestroyInstance() in its destructor. Movable, non-copyable. +class RAK_DLL_EXPORT Peer { +public: + Peer() : raw_(RakPeerInterface::GetInstance()) {} + ~Peer(); + + Peer(Peer&& o) noexcept; + Peer& operator=(Peer&& o) noexcept; + Peer(const Peer&) = delete; + Peer& operator=(const Peer&) = delete; + + RakPeerInterface* operator->() const { return raw_; } + RakPeerInterface* get() const { return raw_; } + /// False only for a moved-from (and thus empty) handle. + explicit operator bool() const { return raw_ != nullptr; } + + /// Receive the next queued packet wrapped in a PacketPtr (may be empty). + PacketPtr receive(); + +private: + RakPeerInterface* raw_; +}; + +} // namespace MafiaNet diff --git a/Source/include/mafianet/mafianet.h b/Source/include/mafianet/mafianet.h index b952c8d2f..ce0cad77a 100644 --- a/Source/include/mafianet/mafianet.h +++ b/Source/include/mafianet/mafianet.h @@ -31,3 +31,4 @@ #include "mafianet/GetTime.h" // MafiaNet::GetTime / TimeMS #include "mafianet/guid_util.h" // MafiaNet::to_string / connected_address #include "mafianet/aliases.h" // canonical aliases: PeerInterface, Guid, Statistics +#include "mafianet/PeerHandle.h" // RAII handles: Peer, PacketPtr diff --git a/Source/src/PeerHandle.cpp b/Source/src/PeerHandle.cpp new file mode 100644 index 000000000..edc9702e1 --- /dev/null +++ b/Source/src/PeerHandle.cpp @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2019, SLikeSoft UG (haftungsbeschraenkt) + * + * This source code is licensed under the MIT-style license found in the license.txt + * file in the root directory of this source tree. + */ + +#include "mafianet/PeerHandle.h" + +#include + +#include "mafianet/MessageIdentifiers.h" // ID_TIMESTAMP + +namespace MafiaNet { + +// --- Compile-time contract: movable, non-copyable (acceptance criterion #1) --- +static_assert(std::is_move_constructible::value, "PacketPtr must be move-constructible"); +static_assert(std::is_move_assignable::value, "PacketPtr must be move-assignable"); +static_assert(!std::is_copy_constructible::value, "PacketPtr must NOT be copy-constructible"); +static_assert(!std::is_copy_assignable::value, "PacketPtr must NOT be copy-assignable"); +static_assert(std::is_move_constructible::value, "Peer must be move-constructible"); +static_assert(std::is_move_assignable::value, "Peer must be move-assignable"); +static_assert(!std::is_copy_constructible::value, "Peer must NOT be copy-constructible"); +static_assert(!std::is_copy_assignable::value, "Peer must NOT be copy-assignable"); + +// --- PacketPtr --- + +PacketPtr::~PacketPtr() { + if (p_) + owner_->DeallocatePacket(p_); +} + +PacketPtr::PacketPtr(PacketPtr&& o) noexcept : owner_(o.owner_), p_(o.p_) { + o.p_ = nullptr; +} + +PacketPtr& PacketPtr::operator=(PacketPtr&& o) noexcept { + if (this != &o) { + if (p_) + owner_->DeallocatePacket(p_); + owner_ = o.owner_; + p_ = o.p_; + o.p_ = nullptr; + } + return *this; +} + +unsigned char PacketPtr::id() const { + // Mirrors GetPacketIdentifier from the samples: an ID_TIMESTAMP prefix is + // followed by the MessageID + Time, then the real identifier byte. + if (p_ == nullptr || p_->length == 0) + return 255; + + if (static_cast(p_->data[0]) == ID_TIMESTAMP) { + // Guard against a truncated timestamp packet: relying on RakAssert alone + // would be an out-of-bounds read in release builds. + RakAssert(p_->length > sizeof(MessageID) + sizeof(Time)); + if (p_->length <= sizeof(MessageID) + sizeof(Time)) + return 255; + return static_cast(p_->data[sizeof(MessageID) + sizeof(Time)]); + } + return static_cast(p_->data[0]); +} + +// --- Peer --- + +Peer::~Peer() { + if (raw_) + RakPeerInterface::DestroyInstance(raw_); +} + +Peer::Peer(Peer&& o) noexcept : raw_(o.raw_) { + o.raw_ = nullptr; +} + +Peer& Peer::operator=(Peer&& o) noexcept { + if (this != &o) { + if (raw_) + RakPeerInterface::DestroyInstance(raw_); + raw_ = o.raw_; + o.raw_ = nullptr; + } + return *this; +} + +PacketPtr Peer::receive() { + return PacketPtr(raw_, raw_->Receive()); +} + +} // namespace MafiaNet