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.
This commit is contained in:
Stephen Seo 2020-04-15 19:49:17 +09:00
parent e01a1ccd94
commit cf6ff5a040
4 changed files with 87 additions and 10 deletions

View file

@ -147,15 +147,19 @@ typedef struct {
/*! /*!
* \brief Data representing a received/sent packet * \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 { typedef struct {
/*! /*!
* A char array of size \ref UDPC_PACKET_MAX_SIZE. Note that the received * A char array of size \ref dataSize. Will be NULL if this UDPC_PacketInfo
* data will probably use up less data than the full size of the array. The * is invalid.
* actual size of the received data is \ref dataSize.
*/ */
// id is stored at offset 8, size 4 (uint32_t) even for "empty" PktInfos // 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 * \brief Flags indication some additional information about the received
* packet. * packet.
@ -169,11 +173,12 @@ typedef struct {
uint32_t flags; uint32_t flags;
/*! /*!
* \brief The size in bytes of the received packet's data inside the \ref data * \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 * 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 * 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 dataSize;
uint16_t rtt; 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); 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); 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_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); int UDPC_set_libsodium_key_easy(UDPC_HContext ctx, const unsigned char *sk);

View file

@ -105,6 +105,7 @@ struct ConnectionData {
bool isUsingLibsodium, bool isUsingLibsodium,
unsigned char *sk, unsigned char *sk,
unsigned char *pk); unsigned char *pk);
~ConnectionData();
// copy // copy
ConnectionData(const ConnectionData& other) = delete; ConnectionData(const ConnectionData& other) = delete;
@ -156,6 +157,7 @@ struct ConnectionData {
struct Context { struct Context {
public: public:
Context(bool isThreaded); Context(bool isThreaded);
~Context();
bool willLog(UDPC_LoggingType); bool willLog(UDPC_LoggingType);

View file

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

View file

@ -309,6 +309,7 @@ int main(int argc, char **argv) {
} }
// printf("Got rtt %u\n", received.rtt); // printf("Got rtt %u\n", received.rtt);
} }
UDPC_free_PacketInfo(received);
} while (size > 0); } while (size > 0);
} }
do { do {