Skip to content

Commit

Permalink
Synchronize on the manager in RepositoryHelper#createMemoryComposite
Browse files Browse the repository at this point in the history
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
  • Loading branch information
laeubi committed Dec 16, 2023
1 parent ba8f64a commit a5652b6
Showing 1 changed file with 26 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down

0 comments on commit a5652b6

Please sign in to comment.