Change how UDPC_PacketInfo handles it's data

The "data" member variable in UDPC_PacketInfo is now handled as a
pointer to dynamic data, instead of an array with a fixed size. Every
time a UDPC_PacketInfo is received from the context,
UDPC_free_PacketInfo() must be called on it to avoid a memory leak.

diff --git a/src/UDPC.h b/src/UDPC.h
index 4393700de91477f2a03f71256795be7d136c430c..b9e1ce33f488f48f34338344f91e5d2f41e8ab3e 100644 (file)
--- a/src/UDPC.h
+++ b/src/UDPC.h
@@ -147,15 +147,19 @@ typedef struct {
 
 /*!
  * \brief Data representing a received/sent packet
+ *
+ * If \ref data is NULL or \ref dataSize is 0, then this packet is invalid.
+ *
+ * \warning This struct must be free'd with a call to UDPC_free_PacketInfo to
+ * avoid a memory leak.
  */
 typedef struct {
     /*!
-     * A char array of size \ref UDPC_PACKET_MAX_SIZE. Note that the received
-     * data will probably use up less data than the full size of the array. The
-     * actual size of the received data is \ref dataSize.
+     * A char array of size \ref dataSize. Will be NULL if this UDPC_PacketInfo
+     * is invalid.
      */
     // id is stored at offset 8, size 4 (uint32_t) even for "empty" PktInfos
-    char data[UDPC_PACKET_MAX_SIZE];
+    char *data;
     /*!
      * \brief Flags indication some additional information about the received
      * packet.
@@ -169,11 +173,12 @@ typedef struct {
     uint32_t flags;
     /*!
      * \brief The size in bytes of the received packet's data inside the \ref data
-     * array member variable.
+     * pointer member variable.
      *
      * UDPC does not return an empty packet when calling UDPC_get_received(), so
      * in such a packet dataSize shouldn't be zero. (UDPC only stores received
-     * packets that do have a payload.)
+     * packets that do have a payload.) This means that if this variable is 0,
+     * then this UDPC_PacketInfo is invalid.
      */
     uint16_t dataSize;
     uint16_t rtt;
@@ -629,8 +634,23 @@ int UDPC_set_receiving_events(UDPC_HContext ctx, int isReceivingEvents);
  */
 UDPC_Event UDPC_get_event(UDPC_HContext ctx, unsigned long *remaining);
 
+/*!
+ * \brief Get a received packet from a given UDPC context.
+ *
+ * \warning The received packet (if valid) must be free'd with a call to
+ * \ref UDPC_free_PacketInfo() to avoid a memory leak.
+ */
 UDPC_PacketInfo UDPC_get_received(UDPC_HContext ctx, unsigned long *remaining);
 
+/*!
+ * \brief Frees a UDPC_PacketInfo.
+ *
+ * Internally, the member variable \ref UDPC_PacketInfo::data will be free'd and
+ * set to NULL and \ref UDPC_PacketInfo::dataSize will be set to 0 if the given
+ * packet is valid.
+ */
+void UDPC_free_PacketInfo(UDPC_PacketInfo pInfo);
+
 int UDPC_set_libsodium_keys(UDPC_HContext ctx, const unsigned char *sk, const unsigned char *pk);
 
 int UDPC_set_libsodium_key_easy(UDPC_HContext ctx, const unsigned char *sk);

diff --git a/src/UDPC_Defines.hpp b/src/UDPC_Defines.hpp
index 416fe1928a49749a6425c6c8afe82fdaaa258099..f97dc12285081d33b96c74dd84ca4cc615d224ca 100644 (file)
--- a/src/UDPC_Defines.hpp
+++ b/src/UDPC_Defines.hpp
@@ -105,6 +105,7 @@ struct ConnectionData {
         bool isUsingLibsodium,
         unsigned char *sk,
         unsigned char *pk);
+    ~ConnectionData();
 
     // copy
     ConnectionData(const ConnectionData& other) = delete;
@@ -156,6 +157,7 @@ struct ConnectionData {
 struct Context {
 public:
     Context(bool isThreaded);
+    ~Context();
 
     bool willLog(UDPC_LoggingType);
 

diff --git a/src/UDPConnection.cpp b/src/UDPConnection.cpp
index 1e0d28fc36fb605e8bca71a78b01b426d03d7a37..0d94ded6a79ff76b0253df7f2bd2bc0f3394c865 100644 (file)
--- a/src/UDPConnection.cpp
+++ b/src/UDPConnection.cpp
@@ -199,6 +199,18 @@ rtt(std::chrono::steady_clock::duration::zero())
 #endif
 }
 
+UDPC::ConnectionData::~ConnectionData() {
+    for(auto iter = sentPkts.begin(); iter != sentPkts.end(); ++iter) {
+        std::free(iter->data);
+    }
+    for(auto iter = sendPkts.begin(); iter != sendPkts.end(); ++iter) {
+        std::free(iter->data);
+    }
+    for(auto iter = priorityPkts.begin(); iter != priorityPkts.end(); ++iter) {
+        std::free(iter->data);
+    }
+}
+
 void UDPC::ConnectionData::cleanupSentPkts() {
     uint32_t id;
     while(sentPkts.size() > UDPC_SENT_PKTS_MAX_SIZE) {
@@ -207,6 +219,7 @@ void UDPC::ConnectionData::cleanupSentPkts() {
         assert(iter != sentInfoMap.end()
                 && "Sent packet must have correspoding entry in sentInfoMap");
         sentInfoMap.erase(iter);
+        std::free(sentPkts.front().data);
         sentPkts.pop_front();
     }
 }
@@ -243,6 +256,24 @@ peerPKWhitelistMutex()
     keysSet.store(false);
 }
 
+UDPC::Context::~Context() {
+    // cleanup packets
+    for(auto iter = receivedPkts.begin();
+             iter != receivedPkts.end();
+           ++iter) {
+        std::free(iter->data);
+    }
+    {
+        auto iter = cSendPkts.begin();
+        auto current = iter.current();
+        while(current) {
+            std::free(current->data);
+            if(!iter.next()) { break; }
+            current = iter.current();
+        }
+    }
+}
+
 bool UDPC::Context::willLog(UDPC_LoggingType type) {
     switch(loggingType.load()) {
     case UDPC_LoggingType::UDPC_SILENT:
@@ -901,6 +932,8 @@ void UDPC::Context::update_impl() {
                 }
 
                 UDPC_PacketInfo pInfo = UDPC::get_empty_pinfo();
+                pInfo.dataSize = UDPC_NSFULL_HEADER_SIZE;
+                pInfo.data = (char*)std::malloc(pInfo.dataSize);
                 pInfo.flags = 0x4;
                 pInfo.sender.addr = in6addr_loopback;
                 pInfo.receiver.addr = iter->first.addr;
@@ -964,6 +997,7 @@ void UDPC::Context::update_impl() {
                             UDPC_atostr((UDPC_HContext)this, iter->first.addr),
                             ", port ",
                             iter->second.port);
+                        std::free(pInfo.data);
                         continue;
                     }
                     std::memcpy(buf.get() + UDPC_MIN_HEADER_SIZE + 1, sig, crypto_sign_BYTES);
@@ -971,6 +1005,7 @@ void UDPC::Context::update_impl() {
                     assert(!"libsodium disabled, invalid state");
                     UDPC_CHECK_LOG(this, UDPC_LoggingType::UDPC_ERROR,
                         "libsodium is disabled, cannot send packet");
+                    std::free(pInfo.data);
                     continue;
 #endif
                 } else {
@@ -1001,15 +1036,17 @@ void UDPC::Context::update_impl() {
                         UDPC_atostr((UDPC_HContext)this, iter->first.addr),
                         ", port = ",
                         iter->second.port);
+                    std::free(pInfo.data);
                     continue;
                 }
 
                 if((pInfo.flags & 0x4) == 0) {
                     // is check-received, store data in case packet gets lost
                     UDPC_PacketInfo sentPInfo = UDPC::get_empty_pinfo();
+                    sentPInfo.dataSize = sendSize;
+                    sentPInfo.data = (char*)std::malloc(sentPInfo.dataSize);
                     std::memcpy(sentPInfo.data, buf.get(), sendSize);
                     sentPInfo.flags = 0;
-                    sentPInfo.dataSize = sendSize;
                     sentPInfo.sender.addr = in6addr_loopback;
                     sentPInfo.receiver.addr = iter->first.addr;
                     sentPInfo.sender.port = ntohs(socketInfo.sin6_port);
@@ -1020,8 +1057,9 @@ void UDPC::Context::update_impl() {
                 } else {
                     // is not check-received, only id stored in data array
                     UDPC_PacketInfo sentPInfo = UDPC::get_empty_pinfo();
+                    sentPInfo.dataSize = UDPC_MIN_HEADER_SIZE;
+                    sentPInfo.data = (char*)std::malloc(sentPInfo.dataSize);
                     sentPInfo.flags = 0x4;
-                    sentPInfo.dataSize = 0;
                     sentPInfo.sender.addr = in6addr_loopback;
                     sentPInfo.receiver.addr = iter->first.addr;
                     sentPInfo.sender.port = ntohs(socketInfo.sin6_port);
@@ -1036,6 +1074,7 @@ void UDPC::Context::update_impl() {
                 UDPC::SentPktInfo::Ptr sentPktInfo = std::make_shared<UDPC::SentPktInfo>();
                 sentPktInfo->id = iter->second.lseq - 1;
                 iter->second.sentInfoMap.insert(std::make_pair(sentPktInfo->id, sentPktInfo));
+                std::free(pInfo.data);
             }
             iter->second.sent = now;
         }
@@ -1572,11 +1611,13 @@ void UDPC::Context::update_impl() {
                     UDPC_PacketInfo resendingData = UDPC::get_empty_pinfo();
                     if(pktSigned) {
                         resendingData.dataSize = sentIter->dataSize - UDPC_LSFULL_HEADER_SIZE;
+                        resendingData.data = (char*)std::malloc(resendingData.dataSize);
                         std::memcpy(resendingData.data,
                             sentIter->data + UDPC_LSFULL_HEADER_SIZE,
                             resendingData.dataSize);
                     } else {
                         resendingData.dataSize = sentIter->dataSize - UDPC_NSFULL_HEADER_SIZE;
+                        resendingData.data = (char*)std::malloc(resendingData.dataSize);
                         std::memcpy(resendingData.data,
                             sentIter->data + UDPC_NSFULL_HEADER_SIZE,
                             resendingData.dataSize);
@@ -1649,6 +1690,7 @@ void UDPC::Context::update_impl() {
     if(pktType == 0 && bytes > (int)UDPC_NSFULL_HEADER_SIZE) {
         UDPC_PacketInfo recPktInfo = UDPC::get_empty_pinfo();
         recPktInfo.dataSize = bytes - UDPC_NSFULL_HEADER_SIZE;
+        recPktInfo.data = (char*)std::malloc(recPktInfo.dataSize);
         std::memcpy(recPktInfo.data,
                     recvBuf + UDPC_NSFULL_HEADER_SIZE,
                     recPktInfo.dataSize);
@@ -1668,6 +1710,7 @@ void UDPC::Context::update_impl() {
     } else if(pktType == 1 && bytes > (int)UDPC_LSFULL_HEADER_SIZE) {
         UDPC_PacketInfo recPktInfo = UDPC::get_empty_pinfo();
         recPktInfo.dataSize = bytes - UDPC_LSFULL_HEADER_SIZE;
+        recPktInfo.data = (char*)std::malloc(recPktInfo.dataSize);
         std::memcpy(recPktInfo.data,
                     recvBuf + UDPC_LSFULL_HEADER_SIZE,
                     recPktInfo.dataSize);
@@ -1798,7 +1841,7 @@ float UDPC::timePointsToFSec(
 
 UDPC_PacketInfo UDPC::get_empty_pinfo() {
     return UDPC_PacketInfo {
-        {0},     // data (array)
+        0,       // data (ptr)
         0,       // flags
         0,       // dataSize
         0,       // rtt
@@ -2084,10 +2127,12 @@ int UDPC_is_valid_context(UDPC_HContext ctx) {
 void UDPC_destroy(UDPC_HContext ctx) {
     UDPC::Context *UDPC_ctx = UDPC::verifyContext(ctx);
     if(UDPC_ctx) {
+        // stop thread if threaded
         if(UDPC_ctx->flags.test(0)) {
             UDPC_ctx->threadRunning.store(false);
             UDPC_ctx->thread.join();
         }
+
 #if UDPC_PLATFORM == UDPC_PLATFORM_WINDOWS
         WSACleanup();
 #endif
@@ -2138,8 +2183,9 @@ void UDPC_queue_send(UDPC_HContext ctx, UDPC_ConnectionId destinationId,
     }
 
     UDPC_PacketInfo sendInfo = UDPC::get_empty_pinfo();
-    std::memcpy(sendInfo.data, data, size);
     sendInfo.dataSize = size;
+    sendInfo.data = (char*)std::malloc(sendInfo.dataSize);
+    std::memcpy(sendInfo.data, data, size);
     sendInfo.sender.addr = in6addr_loopback;
     sendInfo.sender.port = ntohs(c->socketInfo.sin6_port);
     sendInfo.receiver.addr = destinationId.addr;
@@ -2335,6 +2381,14 @@ UDPC_PacketInfo UDPC_get_received(UDPC_HContext ctx, unsigned long *remaining) {
     }
 }
 
+void UDPC_free_PacketInfo(UDPC_PacketInfo pInfo) {
+    if(pInfo.data && pInfo.dataSize > 0) {
+        std::free(pInfo.data);
+        pInfo.data = 0;
+        pInfo.dataSize = 0;
+    }
+}
+
 int UDPC_set_libsodium_keys(UDPC_HContext ctx, const unsigned char *sk, const unsigned char *pk) {
     UDPC::Context *c = UDPC::verifyContext(ctx);
     if(!c || !c->flags.test(2)) {

diff --git a/src/test/UDPC_NetworkTest.c b/src/test/UDPC_NetworkTest.c
index 56a65f5e2421d90f6773296be521f88ba39313ba..7b6c2db342f68b13db83d9c2597484b9e7f88717 100644 (file)
--- a/src/test/UDPC_NetworkTest.c
+++ b/src/test/UDPC_NetworkTest.c
@@ -309,6 +309,7 @@ int main(int argc, char **argv) {
                     }
 //                    printf("Got rtt %u\n", received.rtt);
                 }
+                UDPC_free_PacketInfo(received);
             } while (size > 0);
         }
         do {