diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java index 39fd2d1ef44..1f80dc8e3eb 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/P2TargetUtils.java @@ -195,6 +195,22 @@ public class P2TargetUtils { */ private static final Map SYNCHRONIZERS = new WeakHashMap<>(); + /** + * Locks for profile operations, keyed by profile ID. This ensures that operations + * on the same profile (which can be shared by multiple ITargetDefinition instances) + * are properly synchronized to prevent timestamp conflicts. + *

+ * Lock objects persist for the lifetime of the application, which is acceptable because: + *

+ *

+ */ + private static final ConcurrentHashMap PROFILE_LOCKS = new ConcurrentHashMap<>(); + /** * Table mapping of ITargetDefinition and IFileArtifactRepository */ @@ -799,6 +815,41 @@ public static IQueryResult getIUs(ITargetDefinition target, IP return synchronizer.getProfile().query(QueryUtil.createIUAnyQuery(), null); } + /** + * Gets or creates a lock object for the given profile ID. This ensures that + * operations on the same profile (which can be shared by multiple ITargetDefinition + * instances) are properly synchronized. + *

+ * Note: Lock objects accumulate in memory but this is acceptable since profile IDs + * are based on stable target file paths and the lock objects are small. + *

+ * + * @param profileId the profile identifier + * @return a lock object for the profile + */ + private static Object getProfileLock(String profileId) { + return PROFILE_LOCKS.computeIfAbsent(profileId, k -> new Object()); + } + + /** + * Refreshes a profile from the registry after operations that modify it. + * The P2 engine creates new profile snapshots with updated timestamps during + * provisioning operations, so the in-memory profile reference must be refreshed. + * + * @param profile the current profile reference (may have outdated timestamp) + * @param target the target definition (used only for error messages) + * @return the refreshed profile from the registry + * @throws CoreException if the profile was removed or cannot be loaded + */ + private IProfile refreshProfile(IProfile profile, ITargetDefinition target) throws CoreException { + String profileId = profile.getProfileId(); + IProfile refreshed = getProfileRegistry().getProfile(profileId); + if (refreshed == null) { + throw new CoreException(Status.error("Profile was removed: " + profileId)); //$NON-NLS-1$ + } + return refreshed; + } + /** * Synchronize the profile and the target definition managed by this synchronizer. On return the profile will * be resolved and correctly match the given target. The IUBundleContainers associated with @@ -807,13 +858,30 @@ public static IQueryResult getIUs(ITargetDefinition target, IP * * NOTE: this is a potentially *very* heavyweight operation. * - * NOTE: this method is synchronized as it is effectively a "test and set" caching method. Two - * threads getting the profile at the same time should not execute concurrently or the profiles - * will get out of sync. + * NOTE: this method uses two levels of synchronization: + *
    + *
  • Instance-level (synchronized method): Protects this P2TargetUtils instance's state (fProfile, etc.)
  • + *
  • Profile-level (profile ID lock): Prevents concurrent access to the same P2 profile from different + * P2TargetUtils instances, which can occur when multiple ITargetDefinition objects refer to the same + * target file and thus share a profile ID
  • + *
* * @throws CoreException if there was a problem synchronizing */ public synchronized void synchronize(ITargetDefinition target, IProgressMonitor monitor) throws CoreException { + // Additional synchronization on the profile ID to prevent race conditions when + // multiple ITargetDefinition instances (with different P2TargetUtils instances) + // share the same profile ID based on the target file path + String profileId = getProfileId(target); + synchronized (getProfileLock(profileId)) { + synchronizeInternal(target, monitor); + } + } + + /** + * Internal implementation of synchronize that runs within both instance and profile locks. + */ + private void synchronizeInternal(ITargetDefinition target, IProgressMonitor monitor) throws CoreException { SubMonitor progress = SubMonitor.convert(monitor, 100); IProfile profile = getProfile(); // Happiness if we have a profile and it checks out or if we can load one and it checks out. @@ -845,6 +913,8 @@ public synchronized void synchronize(ITargetDefinition target, IProgressMonitor } else { resolveWithSlicer(target, profile, progress.split(60)); } + // Refresh the profile from registry as resolve operations may have updated it + profile = refreshProfile(profile, target); fProfile = profile; // If we are updating a profile then delete the old snapshot on success. notify(target, progress.split(15)); @@ -1184,6 +1254,10 @@ public IQueryable getMetadata(IProgressMonitor monitor) { throw new CoreException(result); } + // Refresh the profile from the registry after perform, as the perform operation + // creates a new profile snapshot with an updated timestamp + profile = refreshProfile(profile, target); + // Now that we have a plan with all the binary and explicit bundles, do a second pass and add // in all the source. try {