From 67fdf82b5d6b27719595bc3e84a176bbfaf25f27 Mon Sep 17 00:00:00 2001 From: eazpiazu Date: Fri, 7 Mar 2025 13:18:23 +0100 Subject: [PATCH] Memory leaks corrected and. socket timeout handling improved. --- Source/igtlBindMessage.cxx | 2 ++ Source/igtlMessageHandlerMacro.h | 16 ++++++++-------- Source/igtlNDArrayMessage.cxx | 21 ++++++++++++++++---- Source/igtlNDArrayMessage.h | 4 ++-- Source/igtlSessionManager.cxx | 33 ++++++++++++++++++-------------- Source/igtlSessionManager.h | 1 + Source/igtlSocket.cxx | 27 +++++++++++++++----------- Source/igtlSocket.h | 2 +- 8 files changed, 66 insertions(+), 40 deletions(-) diff --git a/Source/igtlBindMessage.cxx b/Source/igtlBindMessage.cxx index 6c73c9ba..1d867b40 100644 --- a/Source/igtlBindMessage.cxx +++ b/Source/igtlBindMessage.cxx @@ -281,6 +281,8 @@ int BindMessage::UnpackContent() } + igtl_bind_free_info(&bind_info); + return 1; } diff --git a/Source/igtlMessageHandlerMacro.h b/Source/igtlMessageHandlerMacro.h index a6c9b216..ebb2a44a 100644 --- a/Source/igtlMessageHandlerMacro.h +++ b/Source/igtlMessageHandlerMacro.h @@ -37,24 +37,24 @@ using igtl::SmartPointer; igtlTypeMacro(classname, ::igtl::MessageHandler); \ igtlNewMacro(classname); \ public: \ - virtual const char* GetMessageType() \ + virtual const char* GetMessageType() override \ { \ return this->m_Message->GetDeviceType(); \ } \ virtual int Process(messagetype*, datatype*); \ - igtl_uint64 ReceiveMessage(::igtl::Socket* socket, ::igtl::MessageBase* header, int pos) \ + igtl_uint64 ReceiveMessage(::igtl::Socket* socket, ::igtl::MessageBase* header, int pos) override \ { \ if (pos == 0) /* New body */ \ { \ this->m_Message->SetMessageHeader(header); \ this->m_Message->AllocateBuffer(); \ } \ - bool timeout(false); \ + bool error(false); \ igtl_uint64 s = socket->Receive((void*)((char*)this->m_Message->GetBufferBodyPointer()+pos), \ - this->m_Message->GetBufferBodySize()-pos, timeout); \ - if (timeout) /* Time out */ \ + this->m_Message->GetBufferBodySize()-pos, error); \ + if (error) /* Not time out */ \ { \ - return pos; \ + return -1; \ } \ if (s+pos >= this->m_Message->GetBufferBodySize()) \ { \ @@ -114,12 +114,12 @@ using igtl::SmartPointer; { \ return this->m_Message->GetMessageType(); \ } \ - virtual const char* GetMessageType() \ + virtual const char* GetMessageType() override \ { \ return this->m_Message->GetDeviceType(); \ } \ virtual int Process(messagetype*, datatype*); \ - igtl_uint64 ReceiveMessage(::igtl::Socket* socket, ::igtl::MessageBase* header, int pos) \ + igtl_uint64 ReceiveMessage(::igtl::Socket* socket, ::igtl::MessageBase* header, int pos) override \ { \ if (pos == 0) /* New body */ \ { \ diff --git a/Source/igtlNDArrayMessage.cxx b/Source/igtlNDArrayMessage.cxx index b261e9cd..b20fa414 100644 --- a/Source/igtlNDArrayMessage.cxx +++ b/Source/igtlNDArrayMessage.cxx @@ -149,6 +149,11 @@ NDArrayMessage::NDArrayMessage(): NDArrayMessage::~NDArrayMessage() { + if (m_Array != NULL) + { + delete m_Array; + m_Array = NULL; + } } @@ -165,8 +170,13 @@ int NDArrayMessage::SetArray(int type, ArrayBase * a) if (a) { - this->m_Array = a; - return 1; + if (this->m_Array) + { + delete this->m_Array; + } + + this->m_Array = a; + return 1; } else { @@ -174,7 +184,6 @@ int NDArrayMessage::SetArray(int type, ArrayBase * a) } } - igtlUint64 NDArrayMessage::CalculateContentBufferSize() { igtlUint64 dataSize; @@ -231,6 +240,8 @@ int NDArrayMessage::PackContent() memcpy(info.array, this->m_Array->GetRawArray(), this->m_Array->GetRawArraySize()); igtl_ndarray_pack(&info, this->m_Content, IGTL_TYPE_PREFIX_NONE); + igtl_ndarray_free_info(&info); + return 1; } @@ -285,7 +296,9 @@ int NDArrayMessage::UnpackContent() this->m_Array->SetSize(size); memcpy(this->m_Array->GetRawArray(), info.array, this->m_Array->GetRawArraySize()); - + + igtl_ndarray_free_info(&info); + return 1; } diff --git a/Source/igtlNDArrayMessage.h b/Source/igtlNDArrayMessage.h index fb8ee757..12df92a6 100644 --- a/Source/igtlNDArrayMessage.h +++ b/Source/igtlNDArrayMessage.h @@ -36,9 +36,9 @@ class IGTLCommon_EXPORT ArrayBase protected: ArrayBase(); - ~ArrayBase(); public: + virtual ~ArrayBase(); /// Sets the size of the N-D array. Returns non-zero value, if success. int SetSize(IndexType size); @@ -47,7 +47,7 @@ class IGTLCommon_EXPORT ArrayBase IndexType GetSize() { return this->m_Size; }; /// Gets the dimension of the N-D array. - int GetDimension() { return this->m_Size.size(); }; + int GetDimension() { return static_cast(this->m_Size.size()); }; /// Sets an array from a byte array. Size and dimension must be specified prior to /// calling the SetArray() function. diff --git a/Source/igtlSessionManager.cxx b/Source/igtlSessionManager.cxx index ea670440..4ccd459c 100644 --- a/Source/igtlSessionManager.cxx +++ b/Source/igtlSessionManager.cxx @@ -186,8 +186,8 @@ int SessionManager::ProcessMessage() this->m_Header->InitBuffer(); // Receive generic header from the socket - bool timeout(false); - igtl_uint64 r = this->m_Socket->Receive(this->m_Header->GetBufferPointer(), this->m_Header->GetBufferSize(), timeout, 0); + bool error(false); + igtl_uint64 r = this->m_Socket->Receive(this->m_Header->GetBufferPointer(), this->m_Header->GetBufferSize(), error, 0); if (r == 0) { this->m_CurrentReadIndex = 0; @@ -197,7 +197,7 @@ int SessionManager::ProcessMessage() if (r != this->m_Header->GetBufferSize()) { // Only a part of header has arrived. - if (timeout) // timeout + if (error) // Error, not timeout { this->m_CurrentReadIndex = 0; } @@ -213,9 +213,9 @@ int SessionManager::ProcessMessage() else if (this->m_CurrentReadIndex < IGTL_HEADER_SIZE) { // Message transfer was interrupted in the header - bool timeout(false); + bool error(false); igtl_uint64 r = this->m_Socket->Receive((void*)((char*)this->m_Header->GetBufferPointer()+this->m_CurrentReadIndex), - this->m_Header->GetBufferSize()-this->m_CurrentReadIndex, timeout, 0); + this->m_Header->GetBufferSize()-this->m_CurrentReadIndex, error, 0); if (r == 0) { this->m_CurrentReadIndex = 0; @@ -290,15 +290,15 @@ int SessionManager::ProcessMessage() igtl_uint64 r = this->m_CurrentMessageHandler->ReceiveMessage(this->m_Socket, this->m_Header, this->m_CurrentReadIndex-IGTL_HEADER_SIZE); - if (r == this->m_Header->GetBodySizeToRead()) - { - this->m_CurrentReadIndex = 0; - this->m_HeaderDeserialized = 0; - } - else - { - this->m_CurrentReadIndex += IGTL_HEADER_SIZE + r; - } + + if (r == this->m_Header->GetBodySizeToRead() || r == -1 /*error*/) + { + ResetMessageProcessing(); + } + else // Only a part of the body has arrived + { + this->m_CurrentReadIndex = IGTL_HEADER_SIZE + r; + } return 1; } @@ -317,5 +317,10 @@ int SessionManager::PushMessage(MessageBase* message) } } +void SessionManager::ResetMessageProcessing() +{ + this->m_CurrentReadIndex = 0; + this->m_HeaderDeserialized = 0; +} } diff --git a/Source/igtlSessionManager.h b/Source/igtlSessionManager.h index 0d3568a6..0758648d 100644 --- a/Source/igtlSessionManager.h +++ b/Source/igtlSessionManager.h @@ -58,6 +58,7 @@ class IGTLCommon_EXPORT SessionManager: public Object int Disconnect(); int ProcessMessage(); int PushMessage(MessageBase*); + void ResetMessageProcessing(); protected: SessionManager(); diff --git a/Source/igtlSocket.cxx b/Source/igtlSocket.cxx index 9c6beecd..21d4a805 100644 --- a/Source/igtlSocket.cxx +++ b/Source/igtlSocket.cxx @@ -350,11 +350,11 @@ int Socket::Send(const void* data, igtlUint64 length) } //----------------------------------------------------------------------------- -igtlUint64 Socket::Receive(void* data, igtlUint64 length, bool& timeout, int readFully/*=1*/) +igtlUint64 Socket::Receive(void* data, igtlUint64 length, bool& error, int readFully/*=1*/) { if (!this->GetConnected()) { - timeout = false; + error = false; return 0; } @@ -365,16 +365,15 @@ igtlUint64 Socket::Receive(void* data, igtlUint64 length, bool& timeout, int rea #if defined(_WIN32) && !defined(__CYGWIN__) int trys = 0; #endif - - int n = recv(this->m_SocketDescriptor, buffer + total, (length > std::numeric_limits::max() ? std::numeric_limits::max() : length - total), 0); - + int readLength = (length > std::numeric_limits::max() ? std::numeric_limits::max() : length - total); + int n = recv(this->m_SocketDescriptor, buffer + total, readLength, 0); #if defined(_WIN32) && !defined(__CYGWIN__) if(n == 0) { // On long messages, Windows recv sometimes fails with WSAENOBUFS, but // will work if you try again. - int error = WSAGetLastError(); - if ((error == WSAENOBUFS) && (trys++ < 1000)) + int lastError = WSAGetLastError(); + if ((lastError == WSAENOBUFS) && (trys++ < 1000)) { Sleep(1); continue; @@ -385,8 +384,13 @@ igtlUint64 Socket::Receive(void* data, igtlUint64 length, bool& timeout, int rea else if (n < 0) { // TODO: Need to check if this means timeout. - timeout = true; - return 0; + int errCode = WSAGetLastError(); + if (errCode != WSAETIMEDOUT) + { + error = true; + return 0; + } + return total; } #else if(n == 0) // Disconnected @@ -397,13 +401,14 @@ igtlUint64 Socket::Receive(void* data, igtlUint64 length, bool& timeout, int rea else if (n < 0) // Error (including time out) { // TODO: If it is time-out, errno == EAGAIN - timeout = true; + error = true; return 0; } #endif total += n; - } while(readFully && total < length); + + } while(readFully && total < length); return total; } diff --git a/Source/igtlSocket.h b/Source/igtlSocket.h index 7e533d3f..268d3f32 100644 --- a/Source/igtlSocket.h +++ b/Source/igtlSocket.h @@ -89,7 +89,7 @@ class IGTLCommon_EXPORT Socket : public Object /// read from the socket. The readFully flag will be ignored if the timeout is active. /// 0 on error, else number of bytes read is returned. /// On timeout, the timeout variable will be set to true and the return value is ignored - igtlUint64 Receive(void* data, igtlUint64 length, bool& timeout, int readFully=1); + igtlUint64 Receive(void* data, igtlUint64 length, bool& error, int readFully=1); /// Set sending/receiving timeout for the existing socket in millisecond. /// This function should be called after opening the socket.