Correct improper usage of the SubMonitor

From the documentation: > 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.
eclipse-equinox · Nov 29, 2024 · 18ff004 · 18ff004
1 parent b4a50b8
commit 18ff004
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 25 deletions.
diff --git a/...epository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/DownloadJob.java b/...epository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/DownloadJob.java
@@ -56,8 +56,7 @@ 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$
+			SubMonitor subMonitor = SubMonitor.convert(masterMonitor.slice(1), 1);
 			try {
 				IStatus status = repository.getArtifact(request, subMonitor);
 				if (!status.isOK()) {

diff --git a/.../src/org/eclipse/equinox/internal/p2/discovery/compatibility/BundleDiscoveryStrategy.java b/.../src/org/eclipse/equinox/internal/p2/discovery/compatibility/BundleDiscoveryStrategy.java
@@ -42,13 +42,15 @@ 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);
+		SubMonitor subMonitor = SubMonitor.convert(monitor,
+				Messages.BundleDiscoveryStrategy_task_loading_local_extensions,
+				extensions.length == 0 ? 1 : extensions.length);
 		try {
 			if (extensions.length > 0) {
-				processExtensions(SubMonitor.convert(monitor, extensions.length), extensions);
+				processExtensions(subMonitor.split(extensions.length), extensions);
 			}
 		} finally {
-			monitor.done();
+			subMonitor.done();
 		}
 	}
 

diff --git a/...rg/eclipse/equinox/internal/p2/discovery/compatibility/RemoteBundleDiscoveryStrategy.java b/...rg/eclipse/equinox/internal/p2/discovery/compatibility/RemoteBundleDiscoveryStrategy.java
@@ -55,7 +55,7 @@ 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);
 		try {
 			File registryCacheFolder;
 			try {
@@ -74,7 +74,7 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 			} catch (IOException e) {
 				throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_io_failure_temp_storage, e));
 			}
-			if (monitor.isCanceled()) {
+			if (subMonitor.isCanceled()) {
 				return;
 			}
 
@@ -84,7 +84,7 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 				TransportUtil.readResource(new URI(directoryUrl), reader -> {
 					DirectoryParser parser = new DirectoryParser();
 					temp[0] = parser.parse(reader);
-				}, SubMonitor.convert(monitor, ticksTenPercent));
+				}, subMonitor.split(ticksTenPercent));
 				directory = temp[0];
 				if (directory == null) {
 					throw new IllegalStateException();
@@ -96,7 +96,7 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 			} catch (IOException e) {
 				throw new CoreException(new Status(IStatus.ERROR, DiscoveryCore.ID_PLUGIN, Messages.RemoteBundleDiscoveryStrategy_io_failure_discovery_directory, e));
 			}
-			if (monitor.isCanceled()) {
+			if (subMonitor.isCanceled()) {
 				return;
 			}
 			if (directory.getEntries().isEmpty()) {
@@ -110,7 +110,7 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 				List<Future<DownloadBundleJob>> futures = new ArrayList<>();
 				// submit jobs
 				for (Directory.Entry entry : directory.getEntries()) {
-					futures.add(executorService.submit(new DownloadBundleJob(entry, monitor)));
+					futures.add(executorService.submit(new DownloadBundleJob(entry, subMonitor)));
 				}
 				int futureSize = ticksTenPercent * 4 / directory.getEntries().size();
 				// collect job results
@@ -122,19 +122,19 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 								bundleJob = job.get(1L, TimeUnit.SECONDS);
 								break;
 							} catch (TimeoutException e) {
-								if (monitor.isCanceled()) {
+								if (subMonitor.isCanceled()) {
 									return;
 								}
 							}
 						}
 						if (bundleJob.file != null) {
 							bundleFileToDirectoryEntry.put(bundleJob.file, bundleJob.entry);
 						}
-						monitor.worked(futureSize);
+						subMonitor.worked(futureSize);
 					} catch (ExecutionException e) {
 						Throwable cause = e.getCause();
 						if (cause instanceof OperationCanceledException) {
-							monitor.setCanceled(true);
+							subMonitor.setCanceled(true);
 							return;
 						}
 						IStatus status;
@@ -146,7 +146,7 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 						// log errors but continue on
 						LogHelper.log(status);
 					} catch (InterruptedException e) {
-						monitor.setCanceled(true);
+						subMonitor.setCanceled(true);
 						return;
 					}
 				}
@@ -163,7 +163,7 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 					if (extensionPoint != null) {
 						IExtension[] extensions = extensionPoint.getExtensions();
 						if (extensions.length > 0) {
-							processExtensions(SubMonitor.convert(monitor, ticksTenPercent * 3), extensions);
+							processExtensions(subMonitor.split(ticksTenPercent * 3), extensions);
 						}
 					}
 				} finally {
@@ -173,7 +173,7 @@ public void performDiscovery(IProgressMonitor monitor) throws CoreException {
 				registryStrategy = null;
 			}
 		} finally {
-			monitor.done();
+			subMonitor.done();
 		}
 	}
 

diff --git a/...g.eclipse.equinox.p2.discovery/src/org/eclipse/equinox/internal/p2/discovery/Catalog.java b/...g.eclipse.equinox.p2.discovery/src/org/eclipse/equinox/internal/p2/discovery/Catalog.java
@@ -73,10 +73,10 @@ public IStatus performDiscovery(IProgressMonitor monitor) {
 
 		final int totalTicks = 100000;
 		final int discoveryTicks = totalTicks - (totalTicks / 10);
-		monitor.beginTask(Messages.Catalog_task_discovering_connectors, totalTicks);
+		SubMonitor subMonitor = SubMonitor.convert(monitor, Messages.Catalog_task_discovering_connectors, totalTicks);
 		try {
 			for (AbstractDiscoveryStrategy discoveryStrategy : discoveryStrategies) {
-				if (monitor.isCanceled()) {
+				if (subMonitor.isCanceled()) {
 					status.add(Status.CANCEL_STATUS);
 					break;
 				}
@@ -85,15 +85,15 @@ public IStatus performDiscovery(IProgressMonitor monitor) {
 				discoveryStrategy.setCertifications(newCertifications);
 				discoveryStrategy.setTags(newTags);
 				try {
-					discoveryStrategy.performDiscovery(SubMonitor.convert(monitor, discoveryTicks / discoveryStrategies.size()));
+					discoveryStrategy.performDiscovery(subMonitor.split(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();
+			subMonitor.done();
 		}
 		return status;
 	}

diff --git a/...quinox.p2.transport.ecf/src/org/eclipse/equinox/internal/p2/transport/ecf/FileReader.java b/...quinox.p2.transport.ecf/src/org/eclipse/equinox/internal/p2/transport/ecf/FileReader.java
@@ -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

diff --git a/....eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/query/QueryableUpdates.java b/....eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/query/QueryableUpdates.java
@@ -43,24 +43,25 @@ 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())
+				if (subMonitor.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();
+			subMonitor.done();
 		}
 	}
 }