Skip to content

Commit

Permalink
From the documentation:
Browse files Browse the repository at this point in the history 
> Since it is illegal to call beginTask on the same IProgressMonitor
> more than once, the same instance of IProgressMonitor must not be
> passed to convert more than once.

The proper way is to replace calls to IProgressMonitor.beginTask(...)
with SubMonitor.convert(...), keep the SubMonitor as a local variable
and use that from now on.

This amends commit 9009085.
  • Loading branch information
@ptziegler @vogella
ptziegler authored and vogella committed Dec 3, 2024
1 parent 7eb2f50 commit 1a42e02
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 148 deletions.
2 changes: 1 addition & 1 deletion bundles/org.eclipse.equinox.p2.artifact.repository/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.equinox.p2.artifact.repository;singleton:=true
Bundle-Version: 1.5.500.qualifier
Bundle-Version: 1.5.600.qualifier
Bundle-Activator: org.eclipse.equinox.internal.p2.artifact.repository.Activator
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
15 changes: 5 additions & 10 deletions ...epository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/DownloadJob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,12 @@ protected IStatus run(IProgressMonitor jobMonitor) {
if (masterMonitor.isCanceled())
return Status.CANCEL_STATUS;
// process the actual request
SubMonitor subMonitor = SubMonitor.convert(masterMonitor, 1);
subMonitor.beginTask("", 1); //$NON-NLS-1$
try {
IStatus status = repository.getArtifact(request, subMonitor);
if (!status.isOK()) {
synchronized (overallStatus) {
overallStatus.add(status);
}
SubMonitor subMonitor = SubMonitor.convert(masterMonitor.slice(1), 1);
IStatus status = repository.getArtifact(request, subMonitor);
if (!status.isOK()) {
synchronized (overallStatus) {
overallStatus.add(status);
}
} finally {
subMonitor.done();
}
} while (true);

Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.equinox.p2.discovery.compatibility/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Bundle-Name
Bundle-SymbolicName: org.eclipse.equinox.p2.discovery.compatibility;singleton:=true
Bundle-Version: 1.3.500.qualifier
Bundle-Version: 1.3.600.qualifier
Bundle-Vendor: %Bundle-Vendor
Bundle-RequiredExecutionEnvironment: JavaSE-17
Require-Bundle: org.eclipse.core.runtime;bundle-version="3.29.0",
Expand Down
12 changes: 5 additions & 7 deletions .../src/org/eclipse/equinox/internal/p2/discovery/compatibility/BundleDiscoveryStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,11 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
}
IExtensionPoint extensionPoint = getExtensionRegistry().getExtensionPoint(ConnectorDiscoveryExtensionReader.EXTENSION_POINT_ID);
IExtension[] extensions = extensionPoint.getExtensions();
monitor.beginTask(Messages.BundleDiscoveryStrategy_task_loading_local_extensions, extensions.length == 0 ? 1 : extensions.length);
try {
if (extensions.length > 0) {
processExtensions(SubMonitor.convert(monitor, extensions.length), extensions);
}
} finally {
monitor.done();
SubMonitor subMonitor = SubMonitor.convert(monitor,
Messages.BundleDiscoveryStrategy_task_loading_local_extensions,
extensions.length == 0 ? 1 : extensions.length);
if (extensions.length > 0) {
processExtensions(subMonitor.newChild(extensions.length), extensions);
}
}

Expand Down
194 changes: 95 additions & 99 deletions ...rg/eclipse/equinox/internal/p2/discovery/compatibility/RemoteBundleDiscoveryStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,125 +55,121 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {

final int totalTicks = 100000;
final int ticksTenPercent = totalTicks / 10;
monitor.beginTask(Messages.RemoteBundleDiscoveryStrategy_task_remote_discovery, totalTicks);
SubMonitor subMonitor = SubMonitor.convert(monitor, Messages.RemoteBundleDiscoveryStrategy_task_remote_discovery, totalTicks);
File registryCacheFolder;
try {
File registryCacheFolder;
try {
if (temporaryStorage != null && temporaryStorage.exists()) {
delete(temporaryStorage);
}
temporaryStorage = File.createTempFile(RemoteBundleDiscoveryStrategy.class.getSimpleName(), ".tmp"); //$NON-NLS-1$
temporaryStorage.delete();
if (!temporaryStorage.mkdirs()) {
throw new IOException();
}
registryCacheFolder = new File(temporaryStorage, ".rcache"); //$NON-NLS-1$
if (!registryCacheFolder.mkdirs()) {
throw new IOException();
}
} catch (IOException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_io_failure_temp_storage, e));
if (temporaryStorage != null && temporaryStorage.exists()) {
delete(temporaryStorage);
}
if (monitor.isCanceled()) {
return;
}

Directory directory;
try {
final Directory[] temp = new Directory[1];
TransportUtil.readResource(new URI(directoryUrl), reader -> {
DirectoryParser parser = new DirectoryParser();
temp[0] = parser.parse(reader);
}, SubMonitor.convert(monitor, ticksTenPercent));
directory = temp[0];
if (directory == null) {
throw new IllegalStateException();
}
} catch (UnknownHostException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, NLS.bind(Messages.RemoteBundleDiscoveryStrategy_unknown_host_discovery_directory, e.getMessage()), e));
} catch (URISyntaxException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, NLS.bind(Messages.RemoteBundleDiscoveryStrategy_Invalid_source_specified_Error, directoryUrl), e));
} catch (IOException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_io_failure_discovery_directory, e));
temporaryStorage = File.createTempFile(RemoteBundleDiscoveryStrategy.class.getSimpleName(), ".tmp"); //$NON-NLS-1$
temporaryStorage.delete();
if (!temporaryStorage.mkdirs()) {
throw new IOException();
}
if (monitor.isCanceled()) {
return;
registryCacheFolder = new File(temporaryStorage, ".rcache"); //$NON-NLS-1$
if (!registryCacheFolder.mkdirs()) {
throw new IOException();
}
if (directory.getEntries().isEmpty()) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_empty_directory));
} catch (IOException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_io_failure_temp_storage, e));
}
if (subMonitor.isCanceled()) {
return;
}

Directory directory;
try {
final Directory[] temp = new Directory[1];
TransportUtil.readResource(new URI(directoryUrl), reader -> {
DirectoryParser parser = new DirectoryParser();
temp[0] = parser.parse(reader);
}, subMonitor.newChild(ticksTenPercent));
directory = temp[0];
if (directory == null) {
throw new IllegalStateException();
}
} catch (UnknownHostException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, NLS.bind(Messages.RemoteBundleDiscoveryStrategy_unknown_host_discovery_directory, e.getMessage()), e));
} catch (URISyntaxException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, NLS.bind(Messages.RemoteBundleDiscoveryStrategy_Invalid_source_specified_Error, directoryUrl), e));
} catch (IOException e) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_io_failure_discovery_directory, e));
}
if (subMonitor.isCanceled()) {
return;
}
if (directory.getEntries().isEmpty()) {
throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_empty_directory));
}

Map<File, Directory.Entry> bundleFileToDirectoryEntry = new HashMap<>();
Map<File, Directory.Entry> bundleFileToDirectoryEntry = new HashMap<>();

ExecutorService executorService = createExecutorService(directory.getEntries().size());
try {
List<Future<DownloadBundleJob>> futures = new ArrayList<>();
// submit jobs
for (Directory.Entry entry : directory.getEntries()) {
futures.add(executorService.submit(new DownloadBundleJob(entry, monitor)));
}
int futureSize = ticksTenPercent * 4 / directory.getEntries().size();
// collect job results
for (Future<DownloadBundleJob> job : futures) {
try {
DownloadBundleJob bundleJob;
for (;;) {
try {
bundleJob = job.get(1L, TimeUnit.SECONDS);
break;
} catch (TimeoutException e) {
if (monitor.isCanceled()) {
return;
}
ExecutorService executorService = createExecutorService(directory.getEntries().size());
try {
List<Future<DownloadBundleJob>> futures = new ArrayList<>();
// submit jobs
for (Directory.Entry entry : directory.getEntries()) {
futures.add(executorService.submit(new DownloadBundleJob(entry, subMonitor)));
}
int futureSize = ticksTenPercent * 4 / directory.getEntries().size();
// collect job results
for (Future<DownloadBundleJob> job : futures) {
try {
DownloadBundleJob bundleJob;
for (;;) {
try {
bundleJob = job.get(1L, TimeUnit.SECONDS);
break;
} catch (TimeoutException e) {
if (subMonitor.isCanceled()) {
return;
}
}
if (bundleJob.file != null) {
bundleFileToDirectoryEntry.put(bundleJob.file, bundleJob.entry);
}
monitor.worked(futureSize);
} catch (ExecutionException e) {
Throwable cause = e.getCause();
if (cause instanceof OperationCanceledException) {
monitor.setCanceled(true);
return;
}
IStatus status;
if (cause instanceof CoreException) {
status = ((CoreException) cause).getStatus();
} else {
status = new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_unexpectedError, cause);
}
// log errors but continue on
LogHelper.log(status);
} catch (InterruptedException e) {
monitor.setCanceled(true);
}
if (bundleJob.file != null) {
bundleFileToDirectoryEntry.put(bundleJob.file, bundleJob.entry);
}
subMonitor.worked(futureSize);
} catch (ExecutionException e) {
Throwable cause = e.getCause();
if (cause instanceof OperationCanceledException) {
subMonitor.setCanceled(true);
return;
}
IStatus status;
if (cause instanceof CoreException) {
status = ((CoreException) cause).getStatus();
} else {
status = new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_unexpectedError, cause);
}
// log errors but continue on
LogHelper.log(status);
} catch (InterruptedException e) {
subMonitor.setCanceled(true);
return;
}
} finally {
executorService.shutdownNow();
}
} finally {
executorService.shutdownNow();
}

try {
registryStrategy = new DiscoveryRegistryStrategy(new File[] {registryCacheFolder}, new boolean[] {false}, this);
registryStrategy.setBundles(bundleFileToDirectoryEntry);
IExtensionRegistry extensionRegistry = new ExtensionRegistry(registryStrategy, this, this);
try {
registryStrategy = new DiscoveryRegistryStrategy(new File[] {registryCacheFolder}, new boolean[] {false}, this);
registryStrategy.setBundles(bundleFileToDirectoryEntry);
IExtensionRegistry extensionRegistry = new ExtensionRegistry(registryStrategy, this, this);
try {
IExtensionPoint extensionPoint = extensionRegistry.getExtensionPoint(ConnectorDiscoveryExtensionReader.EXTENSION_POINT_ID);
if (extensionPoint != null) {
IExtension[] extensions = extensionPoint.getExtensions();
if (extensions.length > 0) {
processExtensions(SubMonitor.convert(monitor, ticksTenPercent * 3), extensions);
}
IExtensionPoint extensionPoint = extensionRegistry.getExtensionPoint(ConnectorDiscoveryExtensionReader.EXTENSION_POINT_ID);
if (extensionPoint != null) {
IExtension[] extensions = extensionPoint.getExtensions();
if (extensions.length > 0) {
processExtensions(subMonitor.newChild(ticksTenPercent * 3), extensions);
}
} finally {
extensionRegistry.stop(this);
}
} finally {
registryStrategy = null;
extensionRegistry.stop(this);
}
} finally {
monitor.done();
registryStrategy = null;
}
}

Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.equinox.p2.discovery/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %Bundle-Name
Bundle-SymbolicName: org.eclipse.equinox.p2.discovery;singleton:=true
Bundle-Version: 1.3.400.qualifier
Bundle-Version: 1.3.500.qualifier
Bundle-Vendor: %Bundle-Vendor
Bundle-RequiredExecutionEnvironment: JavaSE-17
Require-Bundle: org.eclipse.core.runtime;bundle-version="3.29.0",
Expand Down
36 changes: 16 additions & 20 deletions ...g.eclipse.equinox.p2.discovery/src/org/eclipse/equinox/internal/p2/discovery/Catalog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,28 +73,24 @@ public IStatus performDiscovery(IProgressMonitor monitor) {

final int totalTicks = 100000;
final int discoveryTicks = totalTicks - (totalTicks / 10);
monitor.beginTask(Messages.Catalog_task_discovering_connectors, totalTicks);
try {
for (AbstractDiscoveryStrategy discoveryStrategy : discoveryStrategies) {
if (monitor.isCanceled()) {
status.add(Status.CANCEL_STATUS);
break;
}
discoveryStrategy.setCategories(newCategories);
discoveryStrategy.setItems(newItems);
discoveryStrategy.setCertifications(newCertifications);
discoveryStrategy.setTags(newTags);
try {
discoveryStrategy.performDiscovery(SubMonitor.convert(monitor, discoveryTicks / discoveryStrategies.size()));
} catch (CoreException e) {
status.add(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, NLS.bind(Messages.Catalog_Strategy_failed_Error, discoveryStrategy.getClass().getSimpleName()), e));
}
SubMonitor subMonitor = SubMonitor.convert(monitor, Messages.Catalog_task_discovering_connectors, totalTicks);
for (AbstractDiscoveryStrategy discoveryStrategy : discoveryStrategies) {
if (subMonitor.isCanceled()) {
status.add(Status.CANCEL_STATUS);
break;
}
discoveryStrategy.setCategories(newCategories);
discoveryStrategy.setItems(newItems);
discoveryStrategy.setCertifications(newCertifications);
discoveryStrategy.setTags(newTags);
try {
discoveryStrategy.performDiscovery(subMonitor.newChild(discoveryTicks / discoveryStrategies.size()));
} catch (CoreException e) {
status.add(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, NLS.bind(Messages.Catalog_Strategy_failed_Error, discoveryStrategy.getClass().getSimpleName()), e));
}

update(newCategories, newItems, newCertifications, newTags);
} finally {
monitor.done();
}

update(newCategories, newItems, newCertifications, newTags);
return status;
}

Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.equinox.p2.transport.ecf/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %pluginName
Bundle-SymbolicName: org.eclipse.equinox.p2.transport.ecf
Bundle-Version: 1.4.300.qualifier
Bundle-Version: 1.4.400.qualifier
Bundle-RequiredExecutionEnvironment: JavaSE-17
Require-Bundle: org.eclipse.ecf;bundle-version="3.1.0",
org.eclipse.ecf.filetransfer;bundle-version="4.0.0",
Expand Down
2 changes: 1 addition & 1 deletion ...quinox.p2.transport.ecf/src/org/eclipse/equinox/internal/p2/transport/ecf/FileReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public final class FileReader extends FileTransferJob implements IFileTransferLi
*/
static class SuppressBlockedMonitor extends ProgressMonitorWrapper {
public SuppressBlockedMonitor(IProgressMonitor monitor, int ticks) {
super(SubMonitor.convert(monitor, ticks));
super(monitor.slice(ticks));
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.equinox.p2.ui/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: %bundleName
Bundle-SymbolicName: org.eclipse.equinox.p2.ui;singleton:=true
Bundle-Version: 2.8.600.qualifier
Bundle-Version: 2.8.700.qualifier
Bundle-Activator: org.eclipse.equinox.internal.p2.ui.ProvUIActivator
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
9 changes: 3 additions & 6 deletions ....eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/query/QueryableUpdates.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,21 @@ public IQueryResult<IInstallableUnit> query(IQuery<IInstallableUnit> query, IPro
if (monitor == null)
monitor = new NullProgressMonitor();
int totalWork = 2000;
monitor.beginTask(ProvUIMessages.QueryableUpdates_UpdateListProgress, totalWork);
SubMonitor subMonitor = SubMonitor.convert(monitor, ProvUIMessages.QueryableUpdates_UpdateListProgress,
totalWork);
IPlanner planner = ui.getSession().getProvisioningAgent().getService(IPlanner.class);
try {
Set<IInstallableUnit> allUpdates = new HashSet<>();
for (IInstallableUnit unit : iusToUpdate) {
if (monitor.isCanceled())
return Collector.emptyCollector();
IQueryResult<IInstallableUnit> updates = planner.updatesFor(unit,
new ProvisioningContext(ui.getSession().getProvisioningAgent()),
SubMonitor.convert(monitor, totalWork / 2 / iusToUpdate.length));
subMonitor.split(totalWork / 2 / iusToUpdate.length));
allUpdates.addAll(updates.toUnmodifiableSet());
}
return query.perform(allUpdates.iterator());
} catch (OperationCanceledException e) {
// Nothing more to do, return result
return Collector.emptyCollector();
} finally {
monitor.done();
}
}
}

0 comments on commit 1a42e02

Please sign in to comment.