From e9a654e16edfeca00c5e9f7a46926ba4265eb040 Mon Sep 17 00:00:00 2001 From: SerhiiGadzhilov <71632867+SerhiiGadzhilov@users.noreply.github.com> Date: Wed, 11 Nov 2020 10:01:09 +0300 Subject: [PATCH] SYNERGY-148 Memory leaks on macOS (#6837) * SYNERGY-148 Memory leaks on macOS * SYNEGRY-148 Update ChangeLog and fixed one typo --- ChangeLog | 1 + src/gui/src/MainWindow.cpp | 20 ++---- src/gui/src/MainWindow.h | 2 - src/lib/platform/OSXKeyState.cpp | 62 ++++++----------- src/lib/platform/OSXKeyState.h | 14 ++-- src/lib/platform/OSXMediaKeySimulator.h | 30 -------- src/lib/platform/OSXMediaKeySimulator.m | 92 ------------------------- 7 files changed, 35 insertions(+), 186 deletions(-) delete mode 100644 src/lib/platform/OSXMediaKeySimulator.h delete mode 100644 src/lib/platform/OSXMediaKeySimulator.m diff --git a/ChangeLog b/ChangeLog index 7406a4413..30d77866d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ =========== Bug fixes: - #6831 Incorporating Sonar's major +- #6837 Memory leaks on macOS system Enhancements: diff --git a/src/gui/src/MainWindow.cpp b/src/gui/src/MainWindow.cpp index 843974ae7..728d7c50d 100644 --- a/src/gui/src/MainWindow.cpp +++ b/src/gui/src/MainWindow.cpp @@ -112,7 +112,6 @@ MainWindow::MainWindow (AppConfig& appConfig, m_pSynergy(NULL), m_SynergyState(synergyDisconnected), m_ServerConfig(5, 3, m_AppConfig->screenName(), this), - m_pTempConfigFile(NULL), m_pTrayIcon(NULL), m_pTrayIconMenu(NULL), m_AlreadyHidden(false), @@ -818,17 +817,19 @@ QString MainWindow::configFilename() { // TODO: no need to use a temporary file, since we need it to // be permenant (since it'll be used for Windows services, etc). - m_pTempConfigFile = new QTemporaryFile(); - if (!m_pTempConfigFile->open()) + QTemporaryFile tempConfigFile; + tempConfigFile.setAutoRemove(false); + + if (!tempConfigFile.open()) { QMessageBox::critical(this, tr("Cannot write configuration file"), tr("The temporary configuration file required to start synergy can not be written.")); return ""; } - serverConfig().save(*m_pTempConfigFile); - filename = m_pTempConfigFile->fileName(); + serverConfig().save(tempConfigFile); + filename = tempConfigFile.fileName(); - m_pTempConfigFile->close(); + tempConfigFile.close(); } else { @@ -913,13 +914,6 @@ void MainWindow::stopSynergy() setSynergyState(synergyDisconnected); - // HACK: deleting the object deletes the physical file, which is - // bad, since it could be in use by the Windows service! -#if !defined(Q_OS_WIN) - delete m_pTempConfigFile; -#endif - m_pTempConfigFile = NULL; - // reset so that new connects cause auto-hide. m_AlreadyHidden = false; } diff --git a/src/gui/src/MainWindow.h b/src/gui/src/MainWindow.h index b75a7b6d2..ae0e72f61 100644 --- a/src/gui/src/MainWindow.h +++ b/src/gui/src/MainWindow.h @@ -46,7 +46,6 @@ class QComboBox; class QTabWidget; class QCheckBox; class QRadioButton; -class QTemporaryFile; class QMessageBox; class QAbstractButton; @@ -222,7 +221,6 @@ public slots: QProcess* m_pSynergy; int m_SynergyState; ServerConfig m_ServerConfig; - QTemporaryFile* m_pTempConfigFile; QSystemTrayIcon* m_pTrayIcon; QMenu* m_pTrayIconMenu; bool m_AlreadyHidden; diff --git a/src/lib/platform/OSXKeyState.cpp b/src/lib/platform/OSXKeyState.cpp index 95492340e..57b750e08 100644 --- a/src/lib/platform/OSXKeyState.cpp +++ b/src/lib/platform/OSXKeyState.cpp @@ -253,9 +253,9 @@ OSXKeyState::mapKeyFromEvent(KeyIDs& ids, } // get keyboard info - TISInputSourceRef currentKeyboardLayout = TISCopyCurrentKeyboardLayoutInputSource(); + AutoTISInputSourceRef currentKeyboardLayout(TISCopyCurrentKeyboardLayoutInputSource(), CFRelease); - if (currentKeyboardLayout == NULL) { + if (!currentKeyboardLayout) { return kKeyNone; } @@ -288,7 +288,7 @@ OSXKeyState::mapKeyFromEvent(KeyIDs& ids, } // translate via uchr resource - CFDataRef ref = (CFDataRef) TISGetInputSourceProperty(currentKeyboardLayout, + CFDataRef ref = (CFDataRef) TISGetInputSourceProperty(currentKeyboardLayout.get(), kTISPropertyUnicodeKeyLayoutData); const UCKeyboardLayout* layout = (const UCKeyboardLayout*) CFDataGetBytePtr(ref); const bool layoutValid = (layout != NULL); @@ -397,9 +397,9 @@ OSXKeyState::pollActiveModifiers() const SInt32 OSXKeyState::pollActiveGroup() const { - TISInputSourceRef keyboardLayout = TISCopyCurrentKeyboardLayoutInputSource(); + AutoTISInputSourceRef keyboardLayout(TISCopyCurrentKeyboardLayoutInputSource(), CFRelease); CFDataRef id = (CFDataRef)TISGetInputSourceProperty( - keyboardLayout, kTISPropertyInputSourceID); + keyboardLayout.get(), kTISPropertyInputSourceID); GroupMap::const_iterator i = m_groupMap.find(id); if (i != m_groupMap.end()) { @@ -430,18 +430,19 @@ void OSXKeyState::getKeyMap(synergy::KeyMap& keyMap) { // update keyboard groups + SInt32 numGroups {0}; if (getGroups(m_groups)) { m_groupMap.clear(); - SInt32 numGroups = (SInt32)m_groups.size(); + numGroups = CFArrayGetCount(m_groups.get()); for (SInt32 g = 0; g < numGroups; ++g) { - CFDataRef id = (CFDataRef)TISGetInputSourceProperty( - m_groups[g], kTISPropertyInputSourceID); + TISInputSourceRef keyboardLayout = (TISInputSourceRef)CFArrayGetValueAtIndex(m_groups.get(), g); + CFDataRef id = (CFDataRef)TISGetInputSourceProperty(keyboardLayout, kTISPropertyInputSourceID); m_groupMap[id] = g; } } UInt32 keyboardType = LMGetKbdType(); - for (SInt32 g = 0, n = (SInt32)m_groups.size(); g < n; ++g) { + for (SInt32 g = 0; g < numGroups; ++g) { // add special keys getKeyMapForSpecialKeys(keyMap, g); @@ -450,8 +451,8 @@ OSXKeyState::getKeyMap(synergy::KeyMap& keyMap) // add regular keys // try uchr resource first - CFDataRef resourceRef = (CFDataRef)TISGetInputSourceProperty( - m_groups[g], kTISPropertyUnicodeKeyLayoutData); + TISInputSourceRef keyboardLayout = (TISInputSourceRef)CFArrayGetValueAtIndex(m_groups.get(), g); + CFDataRef resourceRef = (CFDataRef)TISGetInputSourceProperty(keyboardLayout, kTISPropertyUnicodeKeyLayoutData); layoutValid = resourceRef != NULL; if (layoutValid) @@ -834,51 +835,28 @@ OSXKeyState::handleModifierKey(void* target, bool OSXKeyState::getGroups(GroupList& groups) const { - CFIndex n; - bool gotLayouts = false; - // get number of layouts CFStringRef keys[] = { kTISPropertyInputSourceCategory }; CFStringRef values[] = { kTISCategoryKeyboardInputSource }; - CFDictionaryRef dict = CFDictionaryCreate(NULL, (const void **)keys, (const void **)values, 1, NULL, NULL); - CFArrayRef kbds = TISCreateInputSourceList(dict, false); - n = CFArrayGetCount(kbds); - gotLayouts = (n != 0); + AutoCFDictionary dict(CFDictionaryCreate(NULL, (const void **)keys, (const void **)values, 1, NULL, NULL), CFRelease); + GroupList kbds(TISCreateInputSourceList(dict.get(), false), CFRelease); - if (!gotLayouts) { + if (CFArrayGetCount(kbds.get()) > 0) { + groups = std::move(kbds); + } + else{ LOG((CLOG_DEBUG1 "can't get keyboard layouts")); return false; } - // get each layout - groups.clear(); - for (CFIndex i = 0; i < n; ++i) { - bool addToGroups = true; - TISInputSourceRef keyboardLayout = - (TISInputSourceRef)CFArrayGetValueAtIndex(kbds, i); - - if (addToGroups) - groups.push_back(keyboardLayout); - } return true; } void OSXKeyState::setGroup(SInt32 group) { - TISSetInputMethodKeyboardLayoutOverride(m_groups[group]); -} - -void -OSXKeyState::checkKeyboardLayout() -{ - // XXX -- should call this when notified that groups have changed. - // if no notification for that then we should poll. - GroupList groups; - if (getGroups(groups) && groups != m_groups) { - updateKeyMap(); - updateKeyState(); - } + TISInputSourceRef keyboardLayout = (TISInputSourceRef)CFArrayGetValueAtIndex(m_groups.get(), group); + TISSetInputMethodKeyboardLayoutOverride(keyboardLayout); } void diff --git a/src/lib/platform/OSXKeyState.h b/src/lib/platform/OSXKeyState.h index ac9516714..a6277dfd2 100644 --- a/src/lib/platform/OSXKeyState.h +++ b/src/lib/platform/OSXKeyState.h @@ -25,9 +25,9 @@ #include -typedef TISInputSourceRef KeyLayout; class IOSXKeyResource; + //! OS X key state /*! A key state for OS X. @@ -106,7 +106,11 @@ protected: private: class KeyResource; - typedef std::vector GroupList; + typedef void(*CFDeallocator)(CFTypeRef); + typedef std::unique_ptr GroupList; + typedef std::unique_ptr AutoCFDictionary; + typedef std::unique_ptr<__TISInputSource, CFDeallocator> AutoTISInputSourceRef; + // Add hard coded special keys to a synergy::KeyMap. void getKeyMapForSpecialKeys( @@ -122,10 +126,6 @@ private: // Change active keyboard group to group void setGroup(SInt32 group); - // Check if the keyboard layout has changed and update keyboard state - // if so. - void checkKeyboardLayout(); - // Send an event for the given modifier key void handleModifierKey(void* target, UInt32 virtualKey, KeyID id, @@ -170,7 +170,7 @@ private: VirtualKeyMap m_virtualKeyMap; mutable UInt32 m_deadKeyState; - GroupList m_groups; + GroupList m_groups{nullptr, CFRelease}; GroupMap m_groupMap; bool m_shiftPressed; bool m_controlPressed; diff --git a/src/lib/platform/OSXMediaKeySimulator.h b/src/lib/platform/OSXMediaKeySimulator.h deleted file mode 100644 index 751454807..000000000 --- a/src/lib/platform/OSXMediaKeySimulator.h +++ /dev/null @@ -1,30 +0,0 @@ -/* - * synergy -- mouse and keyboard sharing utility - * Copyright (C) 2016 Symless. - * - * This package is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * found in the file COPYING that should have accompanied this file. - * - * This package is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - */ - -#pragma once - -#import - -#include "synergy/key_types.h" - -#if defined(__cplusplus) -extern "C" { -#endif -bool fakeNativeMediaKey(KeyID id); -#if defined(__cplusplus) -} -#endif diff --git a/src/lib/platform/OSXMediaKeySimulator.m b/src/lib/platform/OSXMediaKeySimulator.m deleted file mode 100644 index 646807e31..000000000 --- a/src/lib/platform/OSXMediaKeySimulator.m +++ /dev/null @@ -1,92 +0,0 @@ -/* - * synergy -- mouse and keyboard sharing utility - * Copyright (C) 2016 Symless. - * - * This package is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * found in the file COPYING that should have accompanied this file. - * - * This package is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#import "platform/OSXMediaKeySimulator.h" - -#import - -int convertKeyIDToNXKeyType(KeyID id) -{ - // hidsystem/ev_keymap.h - // NX_KEYTYPE_SOUND_UP 0 - // NX_KEYTYPE_SOUND_DOWN 1 - // NX_KEYTYPE_BRIGHTNESS_UP 2 - // NX_KEYTYPE_BRIGHTNESS_DOWN 3 - // NX_KEYTYPE_MUTE 7 - // NX_KEYTYPE_EJECT 14 - // NX_KEYTYPE_PLAY 16 - // NX_KEYTYPE_NEXT 17 - // NX_KEYTYPE_PREVIOUS 18 - // NX_KEYTYPE_FAST 19 - // NX_KEYTYPE_REWIND 20 - - int type = -1; - switch (id) { - case kKeyAudioUp: - type = 0; - break; - case kKeyAudioDown: - type = 1; - break; - case kKeyBrightnessUp: - type = 2; - break; - case kKeyBrightnessDown: - type = 3; - break; - case kKeyAudioMute: - type = 7; - break; - case kKeyEject: - type = 14; - break; - case kKeyAudioPlay: - type = 16; - break; - case kKeyAudioNext: - type = 17; - break; - case kKeyAudioPrev: - type = 18; - break; - default: - break; - } - - return type; -} - -bool -fakeNativeMediaKey(KeyID id) -{ - - NSEvent* downRef = [NSEvent otherEventWithType:NSSystemDefined - location: NSMakePoint(0, 0) modifierFlags:0xa00 - timestamp:0 windowNumber:0 context:0 subtype:8 - data1:(convertKeyIDToNXKeyType(id) << 16) | ((0xa) << 8) - data2:-1]; - CGEventRef downEvent = [downRef CGEvent]; - - NSEvent* upRef = [NSEvent otherEventWithType:NSSystemDefined - location: NSMakePoint(0, 0) modifierFlags:0xa00 - timestamp:0 windowNumber:0 context:0 subtype:8 - data1:(convertKeyIDToNXKeyType(id) << 16) | ((0xb) << 8) - data2:-1]; - CGEventRef upEvent = [upRef CGEvent]; - - CGEventPost(0, downEvent); - CGEventPost(0, upEvent); - - return true; -}