From 487ef92051991dafee719b871f2496fa6029f1bd Mon Sep 17 00:00:00 2001 From: Simeon Andreev Date: Fri, 27 Jan 2023 13:30:53 +0100 Subject: [PATCH] Prevent CME in LaunchBarManager This change synchronizes access to LaunchBarManager.descriptors, to avoid a ConcurrentModificationException when adding descriptors for 2 launches at the same time. Fixes: #262 --- .../META-INF/MANIFEST.MF | 2 +- .../core/internal/LaunchBarManager.java | 54 ++++++++++++++----- 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/launchbar/org.eclipse.launchbar.core/META-INF/MANIFEST.MF b/launchbar/org.eclipse.launchbar.core/META-INF/MANIFEST.MF index bea01030c16..e176e53f785 100644 --- a/launchbar/org.eclipse.launchbar.core/META-INF/MANIFEST.MF +++ b/launchbar/org.eclipse.launchbar.core/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.launchbar.core;singleton:=true -Bundle-Version: 2.5.0.qualifier +Bundle-Version: 2.5.100.qualifier Bundle-Activator: org.eclipse.launchbar.core.internal.Activator Bundle-Vendor: %providerName Require-Bundle: org.eclipse.core.runtime, diff --git a/launchbar/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/internal/LaunchBarManager.java b/launchbar/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/internal/LaunchBarManager.java index b6d828fbde5..b5be2727799 100644 --- a/launchbar/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/internal/LaunchBarManager.java +++ b/launchbar/org.eclipse.launchbar.core/src/org/eclipse/launchbar/core/internal/LaunchBarManager.java @@ -140,11 +140,13 @@ void init() throws CoreException { ILaunchDescriptor last = null; for (String id : split) { Pair key = toId(id); - ILaunchDescriptor desc = descriptors.get(key); - if (desc != null) { - descriptors.remove(key); - descriptors.put(key, desc); - last = desc; + synchronized (descriptors) { + ILaunchDescriptor desc = descriptors.get(key); + if (desc != null) { + descriptors.remove(key); + descriptors.put(key, desc); + last = desc; + } } } // Set the active desc, with MRU, it should be the last one @@ -304,7 +306,10 @@ private Pair getTargetId(ILaunchTarget target) { } private void addDescriptor(Object launchObject, ILaunchDescriptor descriptor) throws CoreException { - descriptors.put(getDescriptorId(descriptor), descriptor); + Pair descriptorId = getDescriptorId(descriptor); + synchronized (descriptors) { + descriptors.put(descriptorId, descriptor); + } objectDescriptorMap.put(launchObject, descriptor); setActiveLaunchDescriptor(descriptor); } @@ -371,7 +376,10 @@ public void launchObjectRemoved(Object launchObject) throws CoreException { Activator.trace("launch object removed " + launchObject); //$NON-NLS-1$ ILaunchDescriptor descriptor = objectDescriptorMap.remove(launchObject); if (descriptor != null) { - descriptors.remove(getDescriptorId(descriptor)); + Pair descriptorId = getDescriptorId(descriptor); + synchronized (descriptors) { + descriptors.remove(descriptorId); + } if (descriptor.equals(activeLaunchDesc)) { setActiveLaunchDescriptor(getLastUsedDescriptor()); } @@ -413,7 +421,10 @@ public void launchObjectChanged(Object launchObject) throws CoreException { private ILaunchDescriptor getLastUsedDescriptor() { if (descriptors.size() == 0) return null; - ILaunchDescriptor[] descs = descriptors.values().toArray(new ILaunchDescriptor[descriptors.size()]); + ILaunchDescriptor[] descs; + synchronized (descriptors) { + descs = descriptors.values().toArray(new ILaunchDescriptor[descriptors.size()]); + } return descs[descs.length - 1]; } @@ -421,7 +432,10 @@ private ILaunchDescriptor getLastUsedDescriptor() { public ILaunchDescriptor[] getLaunchDescriptors() { // return descriptor in usage order (most used first). UI can sort them // later as it wishes - ArrayList values = new ArrayList<>(descriptors.values()); + ArrayList values = new ArrayList<>(); + synchronized (descriptors) { + values.addAll(descriptors.values()); + } Collections.reverse(values); return values.toArray(new ILaunchDescriptor[values.size()]); } @@ -475,8 +489,14 @@ public void setActiveLaunchDescriptor(ILaunchDescriptor descriptor) throws CoreE Activator.trace("resync for " + descriptor); //$NON-NLS-1$ return; } - if (descriptor != null && !descriptors.containsValue(descriptor)) { - throw new IllegalStateException(Messages.LaunchBarManager_1); + if (descriptor != null) { + boolean isContained; + synchronized (descriptors) { + isContained = descriptors.containsValue(descriptor); + } + if (!isContained) { + throw new IllegalStateException(Messages.LaunchBarManager_1); + } } if (descriptor == null) { // do not set to null unless no descriptors @@ -498,8 +518,10 @@ private void doSetActiveLaunchDescriptor(ILaunchDescriptor descriptor) { if (descriptor != null) { // keeps most used descriptor last Pair id = getDescriptorId(descriptor); - descriptors.remove(id); - descriptors.put(id, descriptor); + synchronized (descriptors) { + descriptors.remove(id); + descriptors.put(id, descriptor); + } } } @@ -508,7 +530,11 @@ private void storeActiveDescriptor(ILaunchDescriptor descriptor) { // Store the desc order, active one is the last one StringBuffer buff = new StringBuffer(); // TODO: this can be very long string - for (Pair key : descriptors.keySet()) { + ArrayList> keys = new ArrayList<>(); + synchronized (descriptors) { + keys.addAll(descriptors.keySet()); + } + for (Pair key : keys) { if (buff.length() > 0) { buff.append(','); }