diff --git a/ChangeLog b/ChangeLog index d025eb20c..5ef6d588b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,7 @@ Bug fixes: - #7149 Address issues with modifiers and dead keys - #7163 Fix compilation issues for FreeBSD - #7172 Memory leaks in language sync and TLS functionality +- #7175 Memory leaks in copy/paste and drag and drop functionality Github Actions: - #7148 Fix unstable build for windows core diff --git a/src/lib/base/Event.cpp b/src/lib/base/Event.cpp index 9e12664c1..f7a3b79f9 100644 --- a/src/lib/base/Event.cpp +++ b/src/lib/base/Event.cpp @@ -43,6 +43,16 @@ Event::Event(Type type, void* target, void* data, Flags flags) : // do nothing } +Event::Event(Type type, void* target, EventData* dataObject) : + m_type(type), + m_target(target), + m_data(nullptr), + m_flags(kNone), + m_dataObject(dataObject) +{ + +} + Event::Type Event::getType() const { diff --git a/src/lib/base/Event.h b/src/lib/base/Event.h index 7e6c292c5..b99eac83d 100644 --- a/src/lib/base/Event.h +++ b/src/lib/base/Event.h @@ -56,13 +56,21 @@ public: The \p data must be POD (plain old data) allocated by malloc(), which means it cannot have a constructor, destructor or be composed of any types that do. For non-POD (normal C++ objects - use \c setDataObject(). + use \c setDataObject() or use appropriate constructor. \p target is the intended recipient of the event. \p flags is any combination of \c Flags. */ Event(Type type, void* target = NULL, void* data = NULL, Flags flags = kNone); + //! Create \c Event with non-POD data + /*! + \p type of the event + \p target is the intended recipient of the event. + \p dataObject with event data + */ + Event(Type type, void* target, EventData* dataObject); + //! @name manipulators //@{ diff --git a/src/lib/client/Client.cpp b/src/lib/client/Client.cpp index 1cda15a60..911999777 100644 --- a/src/lib/client/Client.cpp +++ b/src/lib/client/Client.cpp @@ -72,8 +72,8 @@ Client::Client( m_suspended(false), m_connectOnResume(false), m_events(events), - m_sendFileThread(NULL), - m_writeToDropDirThread(NULL), + m_sendFileThread(nullptr), + m_writeToDropDirThread(nullptr), m_socket(NULL), m_useSecureNetwork(args.m_enableCrypto), m_args(args), @@ -258,9 +258,9 @@ Client::enter(SInt32 xAbs, SInt32 yAbs, UInt32, KeyModifierMask mask, bool) m_screen->mouseMove(xAbs, yAbs); m_screen->enter(mask); - if (m_sendFileThread != NULL) { + if (m_sendFileThread) { StreamChunker::interruptFile(); - m_sendFileThread = NULL; + m_sendFileThread.reset(nullptr); } } @@ -827,9 +827,8 @@ void Client::onFileRecieveCompleted() { if (isReceivedFileSizeValid()) { - m_writeToDropDirThread = new Thread( - new TMethodJob( - this, &Client::writeToDropDirThread)); + auto method = new TMethodJob(this, &Client::writeToDropDirThread); + m_writeToDropDirThread.reset(new Thread(method)); } } @@ -875,14 +874,13 @@ Client::isReceivedFileSizeValid() void Client::sendFileToServer(const char* filename) { - if (m_sendFileThread != NULL) { + if (m_sendFileThread) { StreamChunker::interruptFile(); } - - m_sendFileThread = new Thread( - new TMethodJob( - this, &Client::sendFileThread, - static_cast(const_cast(filename)))); + + auto data = static_cast(const_cast(filename)); + auto method = new TMethodJob(this, &Client::sendFileThread, data); + m_sendFileThread.reset(new Thread(method)); } void @@ -896,7 +894,7 @@ Client::sendFileThread(void* filename) LOG((CLOG_ERR "failed sending file chunks: %s", error.what())); } - m_sendFileThread = NULL; + m_sendFileThread.reset(nullptr); } void diff --git a/src/lib/client/Client.h b/src/lib/client/Client.h index 8bb80fd40..86ebf1631 100644 --- a/src/lib/client/Client.h +++ b/src/lib/client/Client.h @@ -27,6 +27,7 @@ #include "net/NetworkAddress.h" #include "base/EventTypes.h" #include "mt/CondVar.h" +#include class EventQueueTimer; namespace synergy { class Screen; } @@ -234,8 +235,9 @@ private: String m_receivedFileData; DragFileList m_dragFileList; String m_dragFileExt; - Thread* m_sendFileThread; - Thread* m_writeToDropDirThread; + using AutoThread = std::unique_ptr; + AutoThread m_sendFileThread; + AutoThread m_writeToDropDirThread; TCPSocket* m_socket; bool m_useSecureNetwork; bool m_enableClipboard; diff --git a/src/lib/client/ServerProxy.cpp b/src/lib/client/ServerProxy.cpp index 1583aebbc..ae81d96db 100644 --- a/src/lib/client/ServerProxy.cpp +++ b/src/lib/client/ServerProxy.cpp @@ -920,7 +920,7 @@ ServerProxy::dragInfoReceived() void ServerProxy::handleClipboardSendingEvent(const Event& event, void*) { - ClipboardChunk::send(m_stream, event.getData()); + ClipboardChunk::send(m_stream, event.getDataObject()); } void diff --git a/src/lib/net/TCPListenSocket.cpp b/src/lib/net/TCPListenSocket.cpp index 32f2dcf58..069d5798b 100644 --- a/src/lib/net/TCPListenSocket.cpp +++ b/src/lib/net/TCPListenSocket.cpp @@ -152,7 +152,7 @@ TCPListenSocket::serviceListening(ISocketMultiplexerJob* job, return NULL; } if (read) { - m_events->addEvent(Event(m_events->forIListenSocket().connecting(), this, NULL)); + m_events->addEvent(Event(m_events->forIListenSocket().connecting(), this)); // stop polling on this socket until the client accepts return NULL; } diff --git a/src/lib/net/TCPSocket.cpp b/src/lib/net/TCPSocket.cpp index b9f226c73..c80612ea8 100644 --- a/src/lib/net/TCPSocket.cpp +++ b/src/lib/net/TCPSocket.cpp @@ -443,7 +443,7 @@ TCPSocket::sendConnectionFailedEvent(const char* msg) void TCPSocket::sendEvent(Event::Type type) { - m_events->addEvent(Event(type, getEventTarget(), nullptr)); + m_events->addEvent(Event(type, getEventTarget())); } void diff --git a/src/lib/platform/OSXScreen.mm b/src/lib/platform/OSXScreen.mm index 750830d1e..94dac36ab 100644 --- a/src/lib/platform/OSXScreen.mm +++ b/src/lib/platform/OSXScreen.mm @@ -614,30 +614,28 @@ void OSXScreen::getDropTargetThread(void*) { #if defined(MAC_OS_X_VERSION_10_7) - char* cstr = NULL; - // wait for 5 secs for the drop destinaiton string to be filled. UInt32 timeout = ARCH->time() + 5; - + m_dropTarget.clear(); + while (ARCH->time() < timeout) { CFStringRef cfstr = getCocoaDropTarget(); - cstr = CFStringRefToUTF8String(cfstr); + char* cstr = CFStringRefToUTF8String(cfstr); CFRelease(cfstr); if (cstr != NULL) { + LOG((CLOG_DEBUG "drop target: %s", cstr)); + m_dropTarget = cstr; + free(cstr); break; } ARCH->sleep(.1f); } - if (cstr != NULL) { - LOG((CLOG_DEBUG "drop target: %s", cstr)); - m_dropTarget = cstr; - } - else { + if (m_dropTarget.empty()) { LOG((CLOG_ERR "failed to get drop target")); - m_dropTarget.clear(); } + #else LOG((CLOG_WARN "drag drop not supported")); #endif @@ -2069,14 +2067,15 @@ OSXScreen::CFStringRefToUTF8String(CFStringRef aString) } CFIndex length = CFStringGetLength(aString); - CFIndex maxSize = CFStringGetMaximumSizeForEncoding( - length, - kCFStringEncodingUTF8); + CFIndex maxSize = CFStringGetMaximumSizeForEncoding(length, kCFStringEncodingUTF8); char* buffer = (char*)malloc(maxSize); - if (CFStringGetCString(aString, buffer, maxSize, kCFStringEncodingUTF8)) { - return buffer; + + if (!CFStringGetCString(aString, buffer, maxSize, kCFStringEncodingUTF8)) { + free(buffer); + buffer = NULL; } - return NULL; + + return buffer; } void @@ -2100,21 +2099,20 @@ String& OSXScreen::getDraggingFilename() { if (m_draggingStarted) { + m_draggingFilename.clear(); + CFStringRef dragInfo = getDraggedFileURL(); - char* info = NULL; - info = CFStringRefToUTF8String(dragInfo); - if (info == NULL) { - m_draggingFilename.clear(); - } - else { + char* info = CFStringRefToUTF8String(dragInfo); + CFRelease(dragInfo); + + if (info != NULL) { LOG((CLOG_DEBUG "drag info: %s", info)); - CFRelease(dragInfo); - String fileList(info); - m_draggingFilename = fileList; + m_draggingFilename = info; + free(info); } // fake a escape key down and up then left mouse button up - fakeKeyDown(kKeyEscape, 8192, 1, AppUtil::instance().getCurrentLanguageCode()); + fakeKeyDown(kKeyEscape, 8192, 1, AppUtil::instance().getCurrentLanguageCode()); fakeKeyUp(1); fakeMouseButton(kButtonLeft, false); } diff --git a/src/lib/server/ClientProxy1_6.cpp b/src/lib/server/ClientProxy1_6.cpp index deb70e13b..6e649fe86 100644 --- a/src/lib/server/ClientProxy1_6.cpp +++ b/src/lib/server/ClientProxy1_6.cpp @@ -64,7 +64,7 @@ ClientProxy1_6::setClipboard(ClipboardID id, const IClipboard* clipboard) void ClientProxy1_6::handleClipboardSendingEvent(const Event& event, void*) { - ClipboardChunk::send(getStream(), event.getData()); + ClipboardChunk::send(getStream(), event.getDataObject()); } bool diff --git a/src/lib/server/Server.cpp b/src/lib/server/Server.cpp index f8ddd12a8..437e1769e 100644 --- a/src/lib/server/Server.cpp +++ b/src/lib/server/Server.cpp @@ -91,13 +91,13 @@ Server::Server( m_lockedToScreen(false), m_screen(screen), m_events(events), - m_sendFileThread(NULL), - m_writeToDropDirThread(NULL), + m_sendFileThread(nullptr), + m_writeToDropDirThread(nullptr), m_ignoreFileTransfer(false), m_disableLockToScreen(false), m_enableClipboard(true), - m_maximumClipboardSize(INT_MAX), - m_sendDragInfoThread(NULL), + m_maximumClipboardSize(INT_MAX), + m_sendDragInfoThread(nullptr), m_waitDragInfoThread(true), m_args(args) { @@ -1558,7 +1558,7 @@ Server::handleFakeInputEndEvent(const Event&, void*) void Server::handleFileChunkSendingEvent(const Event& event, void*) { - onFileChunkSending(event.getData()); + onFileChunkSending(event.getDataObject()); } void @@ -1862,11 +1862,11 @@ Server::onMouseMovePrimary(SInt32 x, SInt32 y) && m_screen->isDraggingStarted() && m_active != newScreen && m_waitDragInfoThread) { - if (m_sendDragInfoThread == NULL) { - m_sendDragInfoThread = new Thread( + if (!m_sendDragInfoThread) { + m_sendDragInfoThread.reset(new Thread( new TMethodJob( this, - &Server::sendDragInfoThread, newScreen)); + &Server::sendDragInfoThread, newScreen))); } return false; @@ -1909,7 +1909,8 @@ Server::sendDragInfoThread(void* arg) m_dragFileList.clear(); } m_waitDragInfoThread = false; - m_sendDragInfoThread = NULL; + m_sendDragInfoThread.reset(nullptr); + } void @@ -1919,15 +1920,10 @@ Server::sendDragInfo(BaseClientProxy* newScreen) UInt32 fileCount = DragInformation::setupDragInfo(m_dragFileList, infoString); if (fileCount > 0) { - char* info = NULL; - size_t size = infoString.size(); - info = new char[size]; - memcpy(info, infoString.c_str(), size); - LOG((CLOG_DEBUG2 "sending drag information to client")); - LOG((CLOG_DEBUG3 "dragging file list: %s", info)); - LOG((CLOG_DEBUG3 "dragging file list string size: %i", size)); - newScreen->sendDragInfo(fileCount, info, size); + LOG((CLOG_DEBUG3 "dragging file list: %s", infoString.c_str())); + LOG((CLOG_DEBUG3 "dragging file list string size: %i", infoString.size())); + newScreen->sendDragInfo(fileCount, infoString.c_str(), infoString.size()); } } @@ -2060,9 +2056,9 @@ Server::onMouseMoveSecondary(SInt32 dx, SInt32 dy) } while (false); if (jump) { - if (m_sendFileThread != NULL) { + if (m_sendFileThread) { StreamChunker::interruptFile(); - m_sendFileThread = NULL; + m_sendFileThread.reset(nullptr); } SInt32 newX = m_x; @@ -2126,9 +2122,8 @@ void Server::onFileRecieveCompleted() { if (isReceivedFileSizeValid()) { - m_writeToDropDirThread = new Thread( - new TMethodJob( - this, &Server::writeToDropDirThread)); + auto method = new TMethodJob(this, &Server::writeToDropDirThread); + m_writeToDropDirThread.reset(new Thread(method)); } } @@ -2429,10 +2424,9 @@ Server::sendFileToClient(const char* filename) StreamChunker::interruptFile(); } - m_sendFileThread = new Thread( - new TMethodJob( - this, &Server::sendFileThread, - static_cast(const_cast(filename)))); + auto data = static_cast(const_cast(filename)); + auto method = new TMethodJob(this, &Server::sendFileThread, data); + m_sendFileThread.reset(new Thread(method)); } void @@ -2447,7 +2441,7 @@ Server::sendFileThread(void* data) LOG((CLOG_ERR "failed sending file chunks, error: %s", error.what())); } - m_sendFileThread = NULL; + m_sendFileThread.reset(nullptr); } void diff --git a/src/lib/server/Server.h b/src/lib/server/Server.h index 87055a61c..583a76da1 100644 --- a/src/lib/server/Server.h +++ b/src/lib/server/Server.h @@ -33,6 +33,7 @@ #include "common/stdmap.h" #include "common/stdset.h" #include "common/stdvector.h" +#include class BaseClientProxy; class EventQueueTimer; @@ -471,19 +472,20 @@ private: IEventQueue* m_events; // file transfer + using AutoThread = std::unique_ptr; size_t m_expectedFileSize; String m_receivedFileData; DragFileList m_dragFileList; DragFileList m_fakeDragFileList; - Thread* m_sendFileThread; - Thread* m_writeToDropDirThread; + AutoThread m_sendFileThread; + AutoThread m_writeToDropDirThread; String m_dragFileExt; bool m_ignoreFileTransfer; bool m_disableLockToScreen; bool m_enableClipboard; - size_t m_maximumClipboardSize; + size_t m_maximumClipboardSize; - Thread* m_sendDragInfoThread; + AutoThread m_sendDragInfoThread; bool m_waitDragInfoThread; ClientListener* m_clientListener; diff --git a/src/lib/synergy/Chunk.h b/src/lib/synergy/Chunk.h index ea5f7bd5c..9ceaffec1 100644 --- a/src/lib/synergy/Chunk.h +++ b/src/lib/synergy/Chunk.h @@ -18,13 +18,14 @@ #pragma once #include "common/basic_types.h" +#include -class Chunk { +class Chunk : public EventData { public: Chunk(size_t size); Chunk(Chunk const &) =delete; Chunk(Chunk &&) =delete; - ~Chunk(); + ~Chunk() override; Chunk& operator=(Chunk const &) =delete; Chunk& operator=(Chunk &&) =delete; diff --git a/src/lib/synergy/PacketStreamFilter.cpp b/src/lib/synergy/PacketStreamFilter.cpp index ba109dbc6..7f42e8c8d 100644 --- a/src/lib/synergy/PacketStreamFilter.cpp +++ b/src/lib/synergy/PacketStreamFilter.cpp @@ -83,7 +83,7 @@ PacketStreamFilter::read(void* buffer, UInt32 n) if (m_inputShutdown && m_size == 0) { m_events->addEvent(Event(m_events->forIStream().inputShutdown(), - getEventTarget(), NULL)); + getEventTarget())); } return n;