Skip to content

Commit 18120ff

Browse files
committed
Fix plugin update/uninstall process on MacOS
- Unload bundle executable when updating or uninstalling a plugin - Use `std::filesystem` for copy operations
1 parent 4af0649 commit 18120ff

2 files changed

Lines changed: 90 additions & 46 deletions

File tree

Source/Processors/PluginManager/PluginManager.cpp

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,16 @@
3636

3737

3838
static inline void closeHandle(decltype(LoadedLibInfo::handle) handle) {
39-
if (handle) {
39+
if (handle) {
4040
#ifdef _WIN32
41-
FreeLibrary(handle);
41+
reeLibrary(handle);
4242
#elif defined(__APPLE__)
43-
CFRelease(handle);
43+
CF::CFBundleUnloadExecutable(handle);
44+
CF::CFRelease(handle);
4445
#else
45-
dlclose(handle);
46+
dlclose(handle);
4647
#endif
47-
}
48+
}
4849
}
4950

5051

@@ -228,30 +229,34 @@ void PluginManager::loadPlugins(const File &pluginPath) {
228229
*/
229230

230231
int PluginManager::loadPlugin(const String& pluginLoc) {
231-
/*
232-
Load in the selected processor. This takes the
233-
dynamic object (.so) and copies it into RAM
234-
Dynamic linker requires a C-style string, so we
235-
we have to convert first.
236-
*/
237-
const char* processorLocCString = static_cast<const char*>(pluginLoc.toUTF8());
238232

239233
#ifdef _WIN32
240234
HINSTANCE handle;
241235
const wchar_t* processorLocLPCWSTR = pluginLoc.toWideCharPointer();
242236
handle = LoadLibraryW(processorLocLPCWSTR);
243237
#elif defined(__APPLE__)
244-
CF::CFURLRef bundleURL = CF::CFURLCreateFromFileSystemRepresentation(CF::kCFAllocatorDefault,
245-
reinterpret_cast<const CF::UInt8 *>(processorLocCString),
246-
strlen(processorLocCString),
247-
true);
248-
assert(bundleURL);
249-
CF::CFBundleRef handle = CF::CFBundleCreate(CF::kCFAllocatorDefault, bundleURL);
250-
CFRelease(bundleURL);
238+
CF::CFStringRef processorLocCFString = pluginLoc.toCFString();
239+
CF::CFURLRef bundleURL = CF::CFURLCreateWithFileSystemPath(CF::kCFAllocatorDefault,
240+
processorLocCFString,
241+
CF::kCFURLPOSIXPathStyle,
242+
true);
243+
244+
assert(bundleURL);
245+
CF::CFBundleRef handle = CF::CFBundleCreate(CF::kCFAllocatorDefault, bundleURL);
246+
CF::CFRelease(bundleURL);
247+
CF::CFRelease(processorLocCFString);
251248
#else
252249
// Clear errors
253250
dlerror();
254251

252+
/*
253+
Load in the selected processor. This takes the
254+
dynamic object (.so) and copies it into RAM
255+
Dynamic linker requires a C-style string, so we
256+
we have to convert first.
257+
*/
258+
const char* processorLocCString = static_cast<const char*>(pluginLoc.toUTF8());
259+
255260
/*
256261
Changing this to resolve all variables immediately upon loading.
257262
This will provide for quicker testing of the custom
@@ -596,7 +601,7 @@ bool PluginManager::removePlugin(String libName)
596601
{
597602
case Plugin::PROCESSOR:
598603
{
599-
LOGD("Removing processor plugin");
604+
LOGD("Removing processor plugin: ", pInfo.processor.name);
600605
for(int j = 0; j < processorPlugins.size(); j++)
601606
{
602607
if(processorPlugins[j].name == pInfo.processor.name)
@@ -612,7 +617,7 @@ bool PluginManager::removePlugin(String libName)
612617
}
613618
case Plugin::RECORD_ENGINE:
614619
{
615-
LOGD("Removing record engine plugin");
620+
LOGD("Removing record engine plugin: ", pInfo.recordEngine.name);
616621
for(int j = 0; j < recordEnginePlugins.size(); j++)
617622
{
618623
if(recordEnginePlugins[j].name == pInfo.recordEngine.name)
@@ -628,7 +633,7 @@ bool PluginManager::removePlugin(String libName)
628633
}
629634
case Plugin::DATA_THREAD:
630635
{
631-
LOGD("Removing data thread plugin");
636+
LOGD("Removing data thread plugin: ", pInfo.dataThread.name);
632637
for(int j = 0; j < dataThreadPlugins.size(); j++)
633638
{
634639
if(dataThreadPlugins[j].name == pInfo.dataThread.name)
@@ -644,7 +649,7 @@ bool PluginManager::removePlugin(String libName)
644649
}
645650
case Plugin::FILE_SOURCE:
646651
{
647-
LOGD("Removing file source plugin");
652+
LOGD("Removing file source plugin: ", pInfo.fileSource.name);
648653
for(int j = 0; j < fileSourcePlugins.size(); j++)
649654
{
650655
if(fileSourcePlugins[j].name == pInfo.fileSource.name)
@@ -660,7 +665,7 @@ bool PluginManager::removePlugin(String libName)
660665
}
661666
default:
662667
{
663-
LOGE("Inavlid plugin");
668+
LOGE("Invalid plugin");
664669
break;
665670
}
666671
}

Source/UI/PluginInstaller.cpp

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include <Windows.h>
3636
#endif
3737

38+
namespace fs = std::filesystem;
3839

3940
//-----------------------------------------------------------------------
4041
static inline File getPluginsDirectory() {
@@ -996,9 +997,17 @@ void PluginInfoComponent::buttonClicked(Button* button)
996997
if(!uninstallPlugin(pInfo.pluginName))
997998
{
998999
LOGE("Failed to uninstall ", pInfo.displayName);
1000+
AlertWindow::showMessageBoxAsync(AlertWindow::WarningIcon,
1001+
"[Plugin Installer] " + pInfo.displayName,
1002+
"Failed to uninstall " + pInfo.displayName);
9991003
}
10001004
else
1005+
{
10011006
LOGC(pInfo.displayName, " uninstalled successfully!");
1007+
AlertWindow::showMessageBoxAsync(AlertWindow::InfoIcon,
1008+
"[Plugin Installer] " + pInfo.displayName,
1009+
pInfo.displayName + " uninstalled successfully");
1010+
}
10021011
}
10031012
else if (button == &documentationButton)
10041013
{
@@ -1049,7 +1058,7 @@ void PluginInfoComponent::run()
10491058
{
10501059
AlertWindow::showMessageBoxAsync(AlertWindow::WarningIcon,
10511060
"[Plugin Installer] ERROR",
1052-
"Unable to current installed version of " + pInfo.displayName + "... Plugin update failed.");
1061+
"Unable to remove current installed version of " + pInfo.displayName + "... Plugin update failed.");
10531062

10541063
LOGE("Unable to remove current installed version of " + pInfo.displayName + "... Plugin update failed.");
10551064
return;
@@ -1386,7 +1395,7 @@ bool PluginInfoComponent::uninstallPlugin(const String& plugin)
13861395

13871396
//delete plugin file
13881397
File pluginFile = getPluginsDirectory().getChildFile(dllName);
1389-
if(!pluginFile.deleteFile())
1398+
if(!pluginFile.deleteRecursively())
13901399
{
13911400
LOGE("Unable to delete plugin file ", pluginFile.getFullPathName(), " ... Please remove it manually!!");
13921401
return false;
@@ -1502,47 +1511,77 @@ int PluginInfoComponent::downloadPlugin(const String& plugin, const String& vers
15021511
}
15031512
}
15041513

1505-
// Uncompress plugin zip file in temp directory
1506-
String pluginDllPath;
1507-
1514+
// Create temp directory to uncompress the plugin
15081515
File tempDir = File::getSpecialLocation(File::tempDirectory).getChildFile("open-ephys");
15091516
tempDir.createDirectory();
15101517

1511-
pluginZip.uncompressTo(tempDir);
1518+
// Delete any existing files in temp directory
1519+
if (tempDir.getChildFile("plugins").exists())
1520+
tempDir.getChildFile("plugins").deleteRecursively();
15121521

1513-
if(!isDependency)
1522+
if (tempDir.getChildFile("shared").exists())
1523+
tempDir.getChildFile("shared").deleteRecursively();
1524+
1525+
// Uncompress the plugin zip file to temp directory
1526+
juce::Result res = pluginZip.uncompressTo(tempDir);
1527+
1528+
if (res.failed())
15141529
{
1515-
// copy plugin DLL from temp directory to actual location
1516-
bool copySuccess = tempDir.getChildFile("plugins").getChildFile(dllName)
1517-
.copyFileTo(getPluginsDirectory().getChildFile(dllName));
1530+
LOGE("Failed to uncompress plugin zip file: ", res.getErrorMessage());
1531+
tempDir.deleteRecursively();
1532+
pluginFile.deleteFile();
1533+
return 2;
1534+
}
1535+
1536+
String pluginDllPath;
15181537

1519-
File dllFile = getPluginsDirectory().getChildFile(dllName);
1520-
pluginDllPath = dllFile.getFullPathName();
1538+
// copy plugin DLL from temp directory to actual location
1539+
if(!isDependency)
1540+
{
1541+
fs::path tempPluginPath = tempDir.getChildFile("plugins").getFullPathName().toStdString();
1542+
fs::path destPluginPath = getPluginsDirectory().getFullPathName().toStdString();
15211543

1522-
if(!copySuccess && !dllFile.exists())
1544+
// Copy only if plugin file exists
1545+
if(fs::exists(tempPluginPath))
1546+
{
1547+
const auto copyOptions = fs::copy_options::overwrite_existing
1548+
| fs::copy_options::recursive;
1549+
try {
1550+
fs::copy(tempPluginPath, destPluginPath, copyOptions);
1551+
} catch(fs::filesystem_error& e) {
1552+
LOGE("Could not copy plugin files: \"", e.what(), "\"");
1553+
tempDir.deleteRecursively();
1554+
pluginFile.deleteFile();
1555+
return 2;
1556+
}
1557+
}
1558+
else
15231559
{
1524-
LOGE("Unable to copy necessary plugin files!");
1560+
LOGE("Plugin file not found in temp directory!!");
1561+
tempDir.deleteRecursively();
15251562
pluginFile.deleteFile();
15261563
return 2;
15271564
}
1565+
1566+
pluginDllPath = getPluginsDirectory().getChildFile(dllName).getFullPathName();
15281567
}
15291568

15301569
/* Copy shared files
15311570
* Uses C++17's filesystem::copy functionality to allow copying symlinks
15321571
*/
1533-
std::filesystem::path tempSharedPath = tempDir.getChildFile("shared").getFullPathName().toStdString();
1534-
std::filesystem::path destSharedPath = getSharedDirectory().getFullPathName().toStdString();
1572+
fs::path tempSharedPath = tempDir.getChildFile("shared").getFullPathName().toStdString();
1573+
fs::path destSharedPath = getSharedDirectory().getFullPathName().toStdString();
15351574

15361575
// Copy only if shared files exist
1537-
if(std::filesystem::exists(tempSharedPath))
1576+
if(fs::exists(tempSharedPath))
15381577
{
1539-
const auto copyOptions = std::filesystem::copy_options::overwrite_existing
1540-
| std::filesystem::copy_options::recursive
1541-
| std::filesystem::copy_options::copy_symlinks
1578+
const auto copyOptions = fs::copy_options::overwrite_existing
1579+
| fs::copy_options::recursive
1580+
| fs::copy_options::copy_symlinks
15421581
;
15431582
try {
1544-
std::filesystem::copy(tempSharedPath, destSharedPath, copyOptions);
1545-
} catch(std::filesystem::filesystem_error& e) {
1583+
fs::copy(tempSharedPath, destSharedPath, copyOptions);
1584+
} catch(fs::filesystem_error& e) {
15461585
LOGE("Could not copy shared files: \"", e.what(), "\"");
15471586
}
15481587
}

0 commit comments

Comments
 (0)