Fix incorrect SubMonitor usage patterns

resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java

@@ -179,8 +179,6 @@ protected void copyFile(IFileInfo sourceInfo, IFileStore destination, int option
 				Policy.error(EFS.ERROR_EXISTS, NLS.bind(Messages.fileExists, target.filePath), e);
 			} catch (IOException e) {
 				Policy.error(EFS.ERROR_WRITE, NLS.bind(Messages.failedCopy, this.filePath, target.filePath), e);
-			} finally {
-				subMonitor.done();
 			}
 		} else {
 			super.copyFile(sourceInfo, destination, options, monitor);
@@ -457,55 +455,51 @@ public void move(IFileStore destFile, int options, IProgressMonitor monitor) thr
 		File destination = destinationFile.file;
 		boolean overwrite = (options & EFS.OVERWRITE) != 0;
 		SubMonitor subMonitor = SubMonitor.convert(monitor, NLS.bind(Messages.moving, source.getAbsolutePath()), 1);
+		//this flag captures case renaming on a case-insensitive OS, or moving
+		//two equivalent files in an environment that supports symbolic links.
+		//in these cases we NEVER want to delete anything
+		boolean sourceEqualsDest = false;
 		try {
-			//this flag captures case renaming on a case-insensitive OS, or moving
-			//two equivalent files in an environment that supports symbolic links.
-			//in these cases we NEVER want to delete anything
-			boolean sourceEqualsDest = false;
-			try {
-				sourceEqualsDest = isSameFile(source, destination);
-			} catch (IOException e) {
-				String message = NLS.bind(Messages.couldNotMove, source.getAbsolutePath());
-				Policy.error(EFS.ERROR_WRITE, message, e);
-			}
-			if (!sourceEqualsDest && !overwrite && destination.exists()) {
-				String message = NLS.bind(Messages.fileExists, destination.getAbsolutePath());
-				Policy.error(EFS.ERROR_EXISTS, message);
-			}
-			if (source.renameTo(destination)) {
-				// double-check to ensure we really did move
-				// since java.io.File#renameTo sometimes lies
-				if (!sourceEqualsDest && source.exists()) {
-					// XXX: document when this occurs
-					if (destination.exists()) {
-						// couldn't delete the source so remove the destination and throw an error
-						// XXX: if we fail deleting the destination, the destination (root) may still exist
-						new LocalFile(destination).delete(EFS.NONE, null);
-						String message = NLS.bind(Messages.couldnotDelete, source.getAbsolutePath());
-						Policy.error(EFS.ERROR_DELETE, message);
-					}
-					// source exists but destination doesn't so try to copy below
-				} else {
-					// destination.exists() returns false for broken links, this has to be handled explicitly
-					if (!destination.exists() && !destFile.fetchInfo().getAttribute(EFS.ATTRIBUTE_SYMLINK)) {
-						// neither the source nor the destination exist. this is REALLY bad
-						String message = NLS.bind(Messages.failedMove, source.getAbsolutePath(), destination.getAbsolutePath());
-						Policy.error(EFS.ERROR_WRITE, message);
-					}
-					// the move was successful
-					return;
+			sourceEqualsDest = isSameFile(source, destination);
+		} catch (IOException e) {
+			String message = NLS.bind(Messages.couldNotMove, source.getAbsolutePath());
+			Policy.error(EFS.ERROR_WRITE, message, e);
+		}
+		if (!sourceEqualsDest && !overwrite && destination.exists()) {
+			String message = NLS.bind(Messages.fileExists, destination.getAbsolutePath());
+			Policy.error(EFS.ERROR_EXISTS, message);
+		}
+		if (source.renameTo(destination)) {
+			// double-check to ensure we really did move
+			// since java.io.File#renameTo sometimes lies
+			if (!sourceEqualsDest && source.exists()) {
+				// XXX: document when this occurs
+				if (destination.exists()) {
+					// couldn't delete the source so remove the destination and throw an error
+					// XXX: if we fail deleting the destination, the destination (root) may still exist
+					new LocalFile(destination).delete(EFS.NONE, null);
+					String message = NLS.bind(Messages.couldnotDelete, source.getAbsolutePath());
+					Policy.error(EFS.ERROR_DELETE, message);
 				}
+				// source exists but destination doesn't so try to copy below
+			} else {
+				// destination.exists() returns false for broken links, this has to be handled explicitly
+				if (!destination.exists() && !destFile.fetchInfo().getAttribute(EFS.ATTRIBUTE_SYMLINK)) {
+					// neither the source nor the destination exist. this is REALLY bad
+					String message = NLS.bind(Messages.failedMove, source.getAbsolutePath(), destination.getAbsolutePath());
+					Policy.error(EFS.ERROR_WRITE, message);
+				}
+				// the move was successful
+				return;
 			}
-			// for some reason renameTo didn't work
-			if (sourceEqualsDest) {
-				String message = NLS.bind(Messages.couldNotMove, source.getAbsolutePath());
-				Policy.error(EFS.ERROR_WRITE, message, null);
-			}
-			// fall back to default implementation
-			super.move(destFile, options, subMonitor.newChild(1));
-		} finally {
-			subMonitor.done();
 		}
+		// for some reason renameTo didn't work
+		if (sourceEqualsDest) {
+			String message = NLS.bind(Messages.couldNotMove, source.getAbsolutePath());
+			Policy.error(EFS.ERROR_WRITE, message, null);
+		}
+		// fall back to default implementation
+		super.move(destFile, options, subMonitor.newChild(1));
 	}
 
 	private boolean isSameFile(File source, File destination) throws IOException {

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/events/AutoBuildJob.java

@@ -297,7 +297,7 @@ public void preferenceChange(PreferenceChangeEvent event) {
 	public IStatus run(IProgressMonitor monitor) {
 		SubMonitor subMonitor = SubMonitor.convert(monitor, 1);
 		synchronized (this) {
-			if (subMonitor.isCanceled() || isInterrupted()) {
+			if (isInterrupted()) {
 				return canceled();
 			}
 		}

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/File.java

@@ -91,8 +91,6 @@ public void appendContents(InputStream content, int updateFlags, IProgressMonito
 			}
 		} catch (IOException streamCloseIgnored) {
 			// ignore;
-		} finally {
-			subMonitor.done();
 		}
 	}
 
@@ -163,8 +161,6 @@ public void create(InputStream content, int updateFlags, IProgressMonitor monito
 			}
 		} catch (IOException streamCloseIgnored) {
 			// ignore;
-		} finally {
-			subMonitor.done();
 		}
 	}
 
@@ -177,38 +173,34 @@ public void create(InputStream content, boolean force, IProgressMonitor monitor)
 	@Override
 	public void create(byte[] content, int updateFlags, IProgressMonitor monitor) throws CoreException {
 		SubMonitor subMonitor = SubMonitor.convert(monitor, NLS.bind(Messages.resources_creating, getFullPath()), 100);
+		checkValidPath(path, FILE, true);
+		final ISchedulingRule rule = workspace.getRuleFactory().createRule(this);
 		try {
-			checkValidPath(path, FILE, true);
-			final ISchedulingRule rule = workspace.getRuleFactory().createRule(this);
-			try {
-				workspace.prepareOperation(rule, subMonitor.newChild(1));
-				checkCreatable();
-				workspace.beginOperation(true);
-				IFileStore store = getStore();
-				IFileInfo localInfo = create(updateFlags, subMonitor.newChild(40), store);
-				boolean local = content != null;
-				if (local) {
-					try {
-						internalSetContents(content, localInfo, updateFlags, false, subMonitor.newChild(59));
-					} catch (CoreException | OperationCanceledException e) {
-						// CoreException when a problem happened creating the file on disk
-						// OperationCanceledException when the operation of setting contents has been
-						// canceled
-						// In either case delete from the workspace and disk
-						workspace.deleteResource(this);
-						store.delete(EFS.NONE, null);
-						throw e;
-					}
+			workspace.prepareOperation(rule, subMonitor.newChild(1));
+			checkCreatable();
+			workspace.beginOperation(true);
+			IFileStore store = getStore();
+			IFileInfo localInfo = create(updateFlags, subMonitor.newChild(40), store);
+			boolean local = content != null;
+			if (local) {
+				try {
+					internalSetContents(content, localInfo, updateFlags, false, subMonitor.newChild(59));
+				} catch (CoreException | OperationCanceledException e) {
+					// CoreException when a problem happened creating the file on disk
+					// OperationCanceledException when the operation of setting contents has been
+					// canceled
+					// In either case delete from the workspace and disk
+					workspace.deleteResource(this);
+					store.delete(EFS.NONE, null);
+					throw e;
 				}
-				setLocal(local);
-			} catch (OperationCanceledException e) {
-				workspace.getWorkManager().operationCanceled();
-				throw e;
-			} finally {
-				workspace.endOperation(rule, true);
 			}
+			setLocal(local);
+		} catch (OperationCanceledException e) {
+			workspace.getWorkManager().operationCanceled();
+			throw e;
 		} finally {
-			subMonitor.done();
+			workspace.endOperation(rule, true);
 		}
 	}
 
@@ -244,7 +236,6 @@ IFileInfo create(int updateFlags, IProgressMonitor subMonitor, IFileStore store)
 				throw new ResourceException(IResourceStatus.FAILED_WRITE_LOCAL, getFullPath(), message, null);
 			}
 		}
-		subMonitor.done();
 
 		workspace.createResource(this, updateFlags);
 		return localInfo;
@@ -510,39 +501,33 @@ public void setContents(InputStream content, int updateFlags, IProgressMonitor m
 			}
 		} catch (IOException streamCloseIgnored) {
 			// ignore;
-		} finally {
-			subMonitor.done();
 		}
 	}
 	@Override
 	public void setContents(byte[] content, int updateFlags, IProgressMonitor monitor) throws CoreException {
 		String message = NLS.bind(Messages.resources_settingContents, getFullPath());
 		SubMonitor subMonitor = SubMonitor.convert(monitor, message, 100);
+		if (workspace.shouldValidate) {
+			workspace.validateSave(this);
+		}
+		final ISchedulingRule rule = workspace.getRuleFactory().modifyRule(this);
+		SubMonitor newChild = subMonitor.newChild(1);
 		try {
-			if (workspace.shouldValidate) {
-				workspace.validateSave(this);
-			}
-			final ISchedulingRule rule = workspace.getRuleFactory().modifyRule(this);
-			SubMonitor newChild = subMonitor.newChild(1);
-			try {
-				workspace.prepareOperation(rule, newChild);
-				ResourceInfo info = getResourceInfo(false, false);
-				checkAccessible(getFlags(info));
-				workspace.beginOperation(true);
-				IFileInfo fileInfo = getStore().fetchInfo();
-				if (BitMask.isSet(updateFlags, IResource.DERIVED)) {
-					// update of derived flag during IFile.write:
-					info.set(ICoreConstants.M_DERIVED);
-				}
-				internalSetContents(content, fileInfo, updateFlags, false, subMonitor.newChild(99));
-			} catch (OperationCanceledException e) {
-				workspace.getWorkManager().operationCanceled();
-				throw e;
-			} finally {
-				workspace.endOperation(rule, true);
+			workspace.prepareOperation(rule, newChild);
+			ResourceInfo info = getResourceInfo(false, false);
+			checkAccessible(getFlags(info));
+			workspace.beginOperation(true);
+			IFileInfo fileInfo = getStore().fetchInfo();
+			if (BitMask.isSet(updateFlags, IResource.DERIVED)) {
+				// update of derived flag during IFile.write:
+				info.set(ICoreConstants.M_DERIVED);
 			}
+			internalSetContents(content, fileInfo, updateFlags, false, subMonitor.newChild(99));
+		} catch (OperationCanceledException e) {
+			workspace.getWorkManager().operationCanceled();
+			throw e;
 		} finally {
-			subMonitor.done();
+			workspace.endOperation(rule, true);
 		}
 	}
 
@@ -616,7 +601,6 @@ public void setCharset(String newCharset, IProgressMonitor monitor) throws CoreE
 			workspace.getWorkManager().operationCanceled();
 			throw e;
 		} finally {
-			subMonitor.done();
 			workspace.endOperation(rule, true);
 		}
 	}

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Folder.java

@@ -118,7 +118,6 @@ public void create(int updateFlags, boolean local, IProgressMonitor monitor) thr
 			workspace.getWorkManager().operationCanceled();
 			throw e;
 		} finally {
-			subMonitor.done();
 			workspace.endOperation(rule, true);
 		}
 	}

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Project.java

@@ -252,7 +252,6 @@ public void close(IProgressMonitor monitor) throws CoreException {
 			workspace.getWorkManager().operationCanceled();
 			throw e;
 		} finally {
-			subMonitor.done();
 			workspace.endOperation(rule, true);
 		}
 	}
@@ -360,7 +359,6 @@ public void create(IProjectDescription description, int updateFlags, IProgressMo
 			workspace.getWorkManager().operationCanceled();
 			throw e;
 		} finally {
-			subMonitor.done();
 			workspace.endOperation(rule, true);
 		}
 	}
@@ -612,43 +610,39 @@ public void run(IProgressMonitor innerMonitor) throws CoreException {
 				final ISchedulingRule notificationsRule = relaxed ? null : workspace.getRoot();
 				SubMonitor subMonitor = SubMonitor.convert(innerMonitor, 100);
 				try {
-					try {
-						workspace.prepareOperation(notificationsRule, innerMonitor);
-						if (!shouldBuild()) {
-							return;
-						}
-						workspace.beginOperation(true);
-						workspace.aboutToBuild(Project.this, trigger);
-					} finally {
-						workspace.endOperation(notificationsRule, false);
+					workspace.prepareOperation(notificationsRule, innerMonitor);
+					if (!shouldBuild()) {
+						return;
 					}
+					workspace.beginOperation(true);
+					workspace.aboutToBuild(Project.this, trigger);
+				} finally {
+					workspace.endOperation(notificationsRule, false);
+				}
+				try {
+					IStatus result;
+					workspace.prepareOperation(projectBuildRule, innerMonitor);
+					// don't open the tree eagerly because it will be wasted if no build occurs
+					workspace.beginOperation(false);
+					result = workspace.getBuildManager().build(config, trigger, builderName, args,
+							subMonitor.split(100));
+					if (!result.isOK()) {
+						throw new ResourceException(result);
+					}
+				} finally {
+					workspace.endOperation(projectBuildRule, false);
 					try {
-						IStatus result;
-						workspace.prepareOperation(projectBuildRule, innerMonitor);
-						// don't open the tree eagerly because it will be wasted if no build occurs
+						workspace.prepareOperation(notificationsRule, innerMonitor);
+						// don't open the tree eagerly because it will be wasted if no change occurs
 						workspace.beginOperation(false);
-						result = workspace.getBuildManager().build(config, trigger, builderName, args,
-								subMonitor.split(100));
-						if (!result.isOK()) {
-							throw new ResourceException(result);
+						workspace.broadcastBuildEvent(Project.this, IResourceChangeEvent.POST_BUILD, trigger);
+						// building may close the tree, so open it
+						if (workspace.getElementTree().isImmutable()) {
+							workspace.newWorkingTree();
 						}
 					} finally {
-						workspace.endOperation(projectBuildRule, false);
-						try {
-							workspace.prepareOperation(notificationsRule, innerMonitor);
-							// don't open the tree eagerly because it will be wasted if no change occurs
-							workspace.beginOperation(false);
-							workspace.broadcastBuildEvent(Project.this, IResourceChangeEvent.POST_BUILD, trigger);
-							// building may close the tree, so open it
-							if (workspace.getElementTree().isImmutable()) {
-								workspace.newWorkingTree();
-							}
-						} finally {
-							workspace.endOperation(notificationsRule, false);
-						}
+						workspace.endOperation(notificationsRule, false);
 					}
-				} finally {
-					subMonitor.done();
 				}
 			}