From a5652b6967efb0cd0254f566b9924584e70a7433 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= <laeubi@laeubi-soft.de>
Date: Sat, 16 Dec 2023 18:25:37 +0100
Subject: [PATCH] Synchronize on the manager in
 RepositoryHelper#createMemoryComposite

Currently the RepositoryHelper#createMemoryComposite uses the current
time millis to generate a "unique" URI, but this has several issues:

Two threads can enter the method at the same time and then see that this
"unique" id is currently not taken and then get the same id.
It can even happen that both try to create them at the same time, what
then will result in a provision exception what will return null and
results in no composite created at all.

This do the following to mitigate this:
- uses a UUID instead of timestamp where collisions are already unlikely
- synchronize on the manager for the time of test/create the repository
---
 .../repository/helpers/RepositoryHelper.java  | 44 +++++++++++--------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/repository/helpers/RepositoryHelper.java b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/repository/helpers/RepositoryHelper.java
index 14adbad8e8..21ac5bc49e 100644
--- a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/repository/helpers/RepositoryHelper.java
+++ b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/repository/helpers/RepositoryHelper.java
@@ -22,7 +22,6 @@
 import java.util.*;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.LongStream;
 import java.util.stream.Stream;
 import org.eclipse.core.runtime.*;
 import org.eclipse.equinox.internal.p2.core.helpers.LogHelper;
@@ -131,24 +130,33 @@ public static <T> IRepository<T> createMemoryComposite(IProvisioningAgent agent,
 		if (manager == null) {
 			return null;
 		}
-		try {
-			// create a unique URI
-			URI repositoryURI = LongStream.iterate(System.currentTimeMillis(), t -> t + 1)
-					.mapToObj(t -> URI.create("memory:" + t)) //$NON-NLS-1$
-					.dropWhile(manager::contains).findFirst().orElseThrow();
-			IRepository<T> result = manager.createRepository(repositoryURI, repositoryURI.toString(), repositoryType,
-					null);
-			manager.removeRepository(repositoryURI);
-			return result;
-		} catch (ProvisionException e) {
-			LogHelper.log(e);
-			// just return null
-		} catch (IllegalArgumentException e) {
-			if (!(e.getCause() instanceof URISyntaxException)) {
-				throw e; // not thrown by the URI creation above
-			} // else just return null
+		synchronized (manager) {
+			// one must synchronize here because even if we check that the manager does not
+			// contain a repository, it is still possible that two threads perform the
+			// check at the same time! This will result in both threads see "not exits",
+			// then both try to create the repos and one of them will fail
+			URI repositoryURI;
+			do {
+				try {
+					repositoryURI = new URI("memory:" + UUID.randomUUID()); //$NON-NLS-1$
+				} catch (URISyntaxException e) {
+					// if we can't create the URI we are all lost...
+					return null;
+				}
+				// very unlikely but check if the manager already has this one and the generate
+				// a fresh one...
+			} while (manager.contains(repositoryURI));
+
+			try {
+				IRepository<T> result = manager.createRepository(repositoryURI, repositoryURI.toString(),
+						repositoryType, null);
+				manager.removeRepository(repositoryURI);
+				return result;
+			} catch (ProvisionException e) {
+				LogHelper.log(e);
+			}
+			return null;
 		}
-		return null;
 	}
 
 	/**