Attachment 194499 Details for Bug 343256 – minimal fix + tests 4

[patch] minimal fix + tests 4

343256.patch (text/plain), 19.21 KB, created by James Blackburn

on 2011-05-02 10:54:14 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: James Blackburn

Created: 2011-05-02 10:54:14 EDT

Size: 19.21 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.core.resources
>Index: src/org/eclipse/core/internal/events/BuildManager.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.resources/src/org/eclipse/core/internal/events/BuildManager.java,v
>retrieving revision 1.125
>diff -u -r1.125 BuildManager.java
>--- src/org/eclipse/core/internal/events/BuildManager.java	1 Feb 2011 11:37:52 -0000	1.125
>+++ src/org/eclipse/core/internal/events/BuildManager.java	2 May 2011 14:51:45 -0000
>@@ -14,11 +14,6 @@
>  *******************************************************************************/
> package org.eclipse.core.internal.events;
> 
>-import org.eclipse.core.resources.IProject;
>-import org.eclipse.core.runtime.jobs.ISchedulingRule;
>-
>-import org.eclipse.core.resources.IBuildConfiguration;
>-
> import java.util.*;
> import org.eclipse.core.internal.dtree.DeltaDataTree;
> import org.eclipse.core.internal.resources.*;
>@@ -169,6 +164,7 @@
> 			// For incremental builds, grab a pointer to the current state before computing the delta
> 			currentTree = ((trigger == IncrementalProjectBuilder.FULL_BUILD) || clean) ? null : workspace.getElementTree();
> 			int depth = -1;
>+			ISchedulingRule rule = null;
> 			try {
> 				//short-circuit if none of the projects this builder cares about have changed.
> 				if (!needsBuild(currentBuilder, trigger)) {
>@@ -177,6 +173,7 @@
> 					monitor.done();
> 					return;
> 				}
>+				rule = builder.getRule(trigger, args);
> 				String name = currentBuilder.getLabel();
> 				String message;
> 				if (name != null)
>@@ -185,13 +182,27 @@
> 					message = NLS.bind(Messages.events_invoking_1, builder.getProject().getFullPath());
> 				monitor.subTask(message);
> 				hookStartBuild(builder, trigger);
>+				// Make the current tree immutable before releasing the WS lock
>+				if (rule != null && currentTree != null)
>+					workspace.newWorkingTree();
> 				//release workspace lock while calling builders
> 				depth = getWorkManager().beginUnprotected();
>+				// Acquire the rule required for running this builder
>+				if (rule != null) {
>+					Job.getJobManager().beginRule(rule, monitor);
>+					// Now that we've acquired the rule, changes may have been made concurrently, ensure we're pointing at the 
>+					// correct currentTree so delta contains concurrent changes made in areas guarded by the scheduling rule
>+					if (currentTree != null)
>+						currentTree = workspace.getElementTree();
>+				}
> 				//do the build
> 				SafeRunner.run(getSafeRunnable(trigger, args, status, monitor));
> 			} finally {
>+				// Re-acquire the WS lock, then release the scheduling rule
> 				if (depth >= 0)
> 					getWorkManager().endUnprotected(depth);
>+				if (rule != null)
>+					Job.getJobManager().endRule(rule);
> 				// Be sure to clean up after ourselves.
> 				if (clean || currentBuilder.wasForgetStateRequested()) {
> 					currentBuilder.setLastBuiltTree(null);
>Index: src/org/eclipse/core/internal/resources/Workspace.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Workspace.java,v
>retrieving revision 1.244
>diff -u -r1.244 Workspace.java
>--- src/org/eclipse/core/internal/resources/Workspace.java	1 Feb 2011 11:37:52 -0000	1.244
>+++ src/org/eclipse/core/internal/resources/Workspace.java	2 May 2011 14:51:46 -0000
>@@ -163,7 +163,7 @@
> 	 * reads and concurrent writes, write access to the tree is governed
> 	 * by {@link WorkManager}.
> 	 */
>-	protected ElementTree tree;
>+	protected volatile ElementTree tree;
> 
> 	/**
> 	 * This field is used to control access to the workspace tree during
>@@ -440,13 +440,39 @@
> 	 */
> 	private void buildInternal(IBuildConfiguration[] configs, int trigger, boolean buildReferences, IProgressMonitor monitor) throws CoreException {
> 		monitor = Policy.monitorFor(monitor);
>-		final ISchedulingRule rule = getRuleFactory().buildRule();
>+		// Bug 343256 use a relaxed scheduling rule if the config we're building uses a relaxed rule.
>+		// Otherwise fall-back to WR.
>+		boolean relaxed = false;
>+		if (Job.getJobManager().currentRule() == null && configs.length > 0) {
>+			relaxed = true;
>+			for (IBuildConfiguration config : configs) {
>+				ISchedulingRule requested = getBuildManager().getRule(config, trigger, null, null);
>+				if (requested != null && requested.contains(getRoot())) {
>+					relaxed = false;
>+					break;
>+				}
>+			}
>+		}
>+
>+		// PRE + POST_BUILD, and the build itself are allowed to modify resources, so require the current thread's scheduling rule
>+		// to either contain the WR or be null. Therefore, if not null, ensure it contains the WR rule...
>+		final ISchedulingRule rule = relaxed ? null : getRuleFactory().buildRule();
> 		try {
> 			monitor.beginTask("", Policy.opWork); //$NON-NLS-1$
> 			try {
>-				prepareOperation(rule, monitor);
>-				beginOperation(true);
>-				aboutToBuild(this, trigger);
>+				try {
>+					// Must run the PRE_BUILD with the WRule held before acquiring WS lock
>+					// Can remove this if we run notifications without the WS lock held: bug 249951
>+					prepareOperation(rule == null ? getRuleFactory().buildRule() : rule, monitor);
>+					beginOperation(true);
>+					aboutToBuild(this, trigger);
>+				} finally {
>+					if (rule == null) {
>+						endOperation(getRuleFactory().buildRule(), false, monitor);
>+						prepareOperation(rule, monitor);
>+						beginOperation(false);
>+					}
>+				}
> 				IStatus result;
> 				try {
> 
>@@ -485,6 +511,12 @@
> 
> 					result = getBuildManager().build(configs, requestedConfigs, trigger, Policy.subMonitorFor(monitor, Policy.opWork));
> 				} finally {
>+					// Run the POST_BUILD with the WRule held
>+					if (rule == null) {
>+						endOperation(rule, false, monitor);
>+						prepareOperation(getRuleFactory().buildRule(), monitor);
>+						beginOperation(false);							
>+					}
> 					//must fire POST_BUILD if PRE_BUILD has occurred
> 					broadcastBuildEvent(this, IResourceChangeEvent.POST_BUILD, trigger);
> 				}
>@@ -494,7 +526,8 @@
> 				//building may close the tree, but we are still inside an operation so open it
> 				if (tree.isImmutable())
> 					newWorkingTree();
>-				endOperation(rule, false, Policy.subMonitorFor(monitor, Policy.endOpWork));
>+				// Rule will be the build-rule from the POST_BUILD refresh
>+				endOperation(getRuleFactory().buildRule(), false, Policy.subMonitorFor(monitor, Policy.endOpWork));
> 			}
> 		} finally {
> 			monitor.done();
>#P org.eclipse.core.tests.resources
>Index: src/org/eclipse/core/tests/internal/builders/RelaxedSchedRuleBuilderTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/RelaxedSchedRuleBuilderTest.java,v
>retrieving revision 1.2
>diff -u -r1.2 RelaxedSchedRuleBuilderTest.java
>--- src/org/eclipse/core/tests/internal/builders/RelaxedSchedRuleBuilderTest.java	1 Feb 2011 11:37:48 -0000	1.2
>+++ src/org/eclipse/core/tests/internal/builders/RelaxedSchedRuleBuilderTest.java	2 May 2011 14:51:48 -0000
>@@ -10,13 +10,13 @@
>  *******************************************************************************/
> package org.eclipse.core.tests.internal.builders;
> 
>-import java.util.Map;
>+import java.io.ByteArrayInputStream;
>+import java.util.*;
> import junit.framework.Test;
> import junit.framework.TestSuite;
> import org.eclipse.core.resources.*;
> import org.eclipse.core.runtime.*;
>-import org.eclipse.core.runtime.jobs.ISchedulingRule;
>-import org.eclipse.core.runtime.jobs.Job;
>+import org.eclipse.core.runtime.jobs.*;
> import org.eclipse.core.tests.harness.TestBarrier;
> import org.eclipse.core.tests.internal.builders.TestBuilder.BuilderRuleCallback;
> 
>@@ -205,4 +205,269 @@
> 		tb2.waitForStatus(TestBarrier.STATUS_DONE);
> 	}
> 
>+	private HashSet<ISchedulingRule> getRulesAsSet(ISchedulingRule rule) {
>+		HashSet<ISchedulingRule> rules = new HashSet<ISchedulingRule>();
>+		if (rule == null)
>+			return rules;
>+		if (rule instanceof MultiRule)
>+			rules.addAll(Arrays.asList(((MultiRule) rule).getChildren()));
>+		else
>+			rules.add(rule);
>+		return rules;
>+	}
>+
>+	/**
>+	 * As the builder is run with a relaxed scheduling rule, we ensure that any changes made before
>+	 * the build is actually run are present in the delta.
>+	 * Acquiring the scheduling rule must be done outside of the WS lock, so this tests that
>+	 * a change which sneaks in during the window or the build thread acquiring its scheduling 
>+	 * rule, is correctly present in the builder's delta.
>+	 * @throws Exception
>+	 */
>+	public void testBuilderDeltaUsingRelaxedRuleBug343256() throws Exception {
>+		String name = "testBuildDeltaUsingRelaxedRuleBug343256";
>+		setAutoBuilding(false);
>+		final IProject project = getWorkspace().getRoot().getProject(name);
>+		final IFile foo = project.getFile("foo");
>+		create(project, false);
>+
>+		IProjectDescription desc = project.getDescription();
>+		desc.setBuildSpec(new ICommand[] {createCommand(desc, EmptyDeltaBuilder.BUILDER_NAME, "Project1Build1")});
>+		project.setDescription(desc, getMonitor());
>+
>+		// Ensure the builder is instantiated
>+		project.build(IncrementalProjectBuilder.FULL_BUILD, getMonitor());
>+
>+		final TestBarrier tb1 = new TestBarrier(TestBarrier.STATUS_WAIT_FOR_START);
>+
>+		// Create a builder set a null scheduling rule
>+		EmptyDeltaBuilder builder = EmptyDeltaBuilder.getInstance();
>+
>+		// Set the rule call-back
>+		builder.setRuleCallback(new BuilderRuleCallback() {
>+
>+			boolean called = false;
>+
>+			public ISchedulingRule getRule(String name, IncrementalProjectBuilder builder, int trigger, Map args) {
>+				// Remove once Bug 331187 is fixed.
>+				// Currently #getRule is called twice when building a specific build configuration (so as to minimized change in 
>+				// 3.7 end-game.  As this test is trying to provoke a bug in the window between fetching a rule and applying it
>+				// to the build, we don't want to run the first time #getRule is called (in Workspace#build)
>+				if (!called) {
>+					called = true;
>+					return project;
>+				}
>+				// REMOVE
>+				tb1.setStatus(TestBarrier.STATUS_START);
>+				tb1.waitForStatus(TestBarrier.STATUS_WAIT_FOR_RUN);
>+				try {
>+					// Give the resource modification time be queued
>+					Thread.sleep(10);
>+				} catch (InterruptedException e) {
>+					fail();
>+				}
>+				return project;
>+			}
>+
>+			public IProject[] build(int kind, Map<String, String> args, IProgressMonitor monitor) throws CoreException {
>+				// shared scheduling rule
>+				assertTrue(Job.getJobManager().currentRule().equals(project));
>+				// assert that the delta contains the file foo
>+				IResourceDelta delta = getDelta(project);
>+				assertNotNull("1.1", delta);
>+				assertTrue("1.2", delta.getAffectedChildren().length == 1);
>+				assertTrue("1.3", delta.getAffectedChildren()[0].getResource().equals(foo));
>+				tb1.setStatus(TestBarrier.STATUS_DONE);
>+				return super.build(kind, args, monitor);
>+			}
>+		});
>+
>+		// Run the incremental build
>+		Job j = new Job("IProject.build()") {
>+			protected IStatus run(IProgressMonitor monitor) {
>+				try {
>+					getWorkspace().build(new IBuildConfiguration[] {project.getActiveBuildConfig()}, IncrementalProjectBuilder.INCREMENTAL_BUILD, true, monitor);
>+				} catch (CoreException e) {
>+					fail();
>+				}
>+				return Status.OK_STATUS;
>+			}
>+		};
>+		j.schedule();
>+
>+		// Wait for the build to transition to getRule
>+		tb1.waitForStatus(TestBarrier.STATUS_START);
>+		// Modify a file in the project
>+		j = new Job("IProject.build()") {
>+			protected IStatus run(IProgressMonitor monitor) {
>+				tb1.setStatus(TestBarrier.STATUS_WAIT_FOR_RUN);
>+				ensureExistsInWorkspace(foo, new ByteArrayInputStream(new byte[0]));
>+				return Status.OK_STATUS;
>+			}
>+		};
>+		j.schedule();
>+		tb1.waitForStatus(TestBarrier.STATUS_DONE);
>+	}
>+
>+	/**
>+	 * Tests for regression in running the build with reduced scheduling rules.
>+	 * @throws Exception
>+	 */
>+	public void testBug343256() throws Exception {
>+		String name = "testBug343256";
>+		setAutoBuilding(false);
>+		final IProject project = getWorkspace().getRoot().getProject(name);
>+		create(project, false);
>+
>+		IProjectDescription desc = project.getDescription();
>+		desc.setBuildSpec(new ICommand[] {createCommand(desc, EmptyDeltaBuilder.BUILDER_NAME, "Project1Build1"), createCommand(desc, EmptyDeltaBuilder2.BUILDER_NAME, "Project1Build2")});
>+		project.setDescription(desc, getMonitor());
>+
>+		// Ensure the builder is instantiated
>+		project.build(IncrementalProjectBuilder.CLEAN_BUILD, getMonitor());
>+
>+		final TestBarrier tb1 = new TestBarrier(TestBarrier.STATUS_WAIT_FOR_START);
>+		final TestBarrier tb2 = new TestBarrier(TestBarrier.STATUS_WAIT_FOR_START);
>+
>+		// Scheduling rules to returng from #getRule
>+		final ISchedulingRule[] getRules = new ISchedulingRule[2];
>+		final ISchedulingRule[] buildRules = new ISchedulingRule[2];
>+
>+		// Create a builder set a null scheduling rule
>+		EmptyDeltaBuilder builder = EmptyDeltaBuilder.getInstance();
>+		EmptyDeltaBuilder2 builder2 = EmptyDeltaBuilder2.getInstance();
>+
>+		// Set the rule call-back
>+		builder.setRuleCallback(new BuilderRuleCallback() {
>+			public ISchedulingRule getRule(String name, IncrementalProjectBuilder builder, int trigger, Map args) {
>+				tb1.waitForStatus(TestBarrier.STATUS_START);
>+				return getRules[0];
>+			}
>+
>+			public IProject[] build(int kind, Map<String, String> args, IProgressMonitor monitor) throws CoreException {
>+				HashSet<ISchedulingRule> h1 = getRulesAsSet(Job.getJobManager().currentRule());
>+				HashSet<ISchedulingRule> h2 = getRulesAsSet(buildRules[0]);
>+				// shared scheduling rule
>+				assertTrue(h1.equals(h2));
>+				tb1.setStatus(TestBarrier.STATUS_DONE);
>+				return super.build(kind, args, monitor);
>+			}
>+		});
>+		// Set the rule call-back
>+		builder2.setRuleCallback(new BuilderRuleCallback() {
>+			public ISchedulingRule getRule(String name, IncrementalProjectBuilder builder, int trigger, Map<String, String> args) {
>+				tb2.waitForStatus(TestBarrier.STATUS_START);
>+				return getRules[1];
>+			}
>+
>+			public IProject[] build(int kind, Map<String, String> args, IProgressMonitor monitor) throws CoreException {
>+				HashSet<ISchedulingRule> h1 = getRulesAsSet(Job.getJobManager().currentRule());
>+				HashSet<ISchedulingRule> h2 = getRulesAsSet(buildRules[1]);
>+				assertTrue(h1.equals(h2));
>+				tb2.setStatus(TestBarrier.STATUS_DONE);
>+				return super.build(kind, args, monitor);
>+			}
>+		});
>+
>+		Job j;
>+
>+		// Enable for Bug 331187 
>+		//		// IProject.build()
>+		//		j = new Job("IProject.build()") {
>+		//			protected IStatus run(IProgressMonitor monitor) {
>+		//				try {
>+		//					project.build(IncrementalProjectBuilder.FULL_BUILD, monitor);
>+		//				} catch (CoreException e) {
>+		//					fail(e.toString());
>+		//				}
>+		//				return Status.OK_STATUS;
>+		//			}
>+		//		};
>+		//		invokeTestBug343256(project, getRules, buildRules, tb1, tb2, j);
>+
>+		//		// IWorkspace.build()
>+		//		j = new Job("IWorkspace.build()") {
>+		//			protected IStatus run(IProgressMonitor monitor) {
>+		//				try {
>+		//					getWorkspace().build(IncrementalProjectBuilder.FULL_BUILD, monitor);
>+		//				} catch (CoreException e) {
>+		//					fail(e.toString());
>+		//				}
>+		//				return Status.OK_STATUS;
>+		//			}
>+		//		};
>+		//		invokeTestBug343256(project, getRules, buildRules, tb1, tb2, j);
>+
>+		// IWorkspace.build(IBuildConfiguration[],...)
>+		j = new Job("IWorkspace.build(IBuildConfiguration[],...)") {
>+			protected IStatus run(IProgressMonitor monitor) {
>+				try {
>+					getWorkspace().build(new IBuildConfiguration[] {project.getActiveBuildConfig()}, IncrementalProjectBuilder.FULL_BUILD, true, monitor);
>+				} catch (CoreException e) {
>+					fail(e.toString());
>+				}
>+				return Status.OK_STATUS;
>+			}
>+		};
>+		invokeTestBug343256(project, getRules, buildRules, tb1, tb2, j);
>+
>+		// Test Auto-build
>+		//		j = new Job("Auto-build") {
>+		//			protected IStatus run(IProgressMonitor monitor) {
>+		//				try {
>+		//					getWorkspace().build(IncrementalProjectBuilder.CLEAN_BUILD, getMonitor());
>+		//				} catch (CoreException e) {
>+		//					fail(e.toString());
>+		//				}
>+		//				return Status.OK_STATUS;
>+		//			}
>+		//		};
>+		//		// Auto-build
>+		//		setAutoBuilding(true);
>+		//		// Wait for the build to transition
>+		//		invokeTestBug343256(project, getRules, buildRules, tb1, tb2, j);
>+	}
>+
>+	/**
>+	 * Helper method do invoke a set of tests on Bug343256 using the different sets of builder API
>+	 */
>+	private void invokeTestBug343256(IProject project, ISchedulingRule[] getRules, ISchedulingRule[] buildRules, TestBarrier tb1, TestBarrier tb2, Job j) {
>+		// Test 1 - build project sched rule
>+		getRules[0] = getRules[1] = project;
>+		buildRules[0] = buildRules[1] = new MultiRule(new ISchedulingRule[] {getRules[0]});
>+		tb1.setStatus(TestBarrier.STATUS_START);
>+		tb2.setStatus(TestBarrier.STATUS_START);
>+		j.schedule();
>+		tb1.waitForStatus(TestBarrier.STATUS_DONE);
>+		tb2.waitForStatus(TestBarrier.STATUS_DONE);
>+
>+		// Test 2 - build null rule
>+		getRules[0] = getRules[1] = null;
>+		buildRules[0] = buildRules[1] = null;
>+		tb1.setStatus(TestBarrier.STATUS_START);
>+		tb2.setStatus(TestBarrier.STATUS_START);
>+		j.schedule();
>+		tb1.waitForStatus(TestBarrier.STATUS_DONE);
>+		tb2.waitForStatus(TestBarrier.STATUS_DONE);
>+
>+		// Test 3 - build mixed projects
>+		getRules[0] = buildRules[0] = project;
>+		getRules[1] = buildRules[1] = getWorkspace().getRoot().getProject("other");
>+		tb1.setStatus(TestBarrier.STATUS_START);
>+		tb2.setStatus(TestBarrier.STATUS_START);
>+		j.schedule();
>+		tb1.waitForStatus(TestBarrier.STATUS_DONE);
>+		tb2.waitForStatus(TestBarrier.STATUS_DONE);
>+
>+		// Test 4 - build project + null
>+		getRules[0] = buildRules[0] = project;
>+		getRules[1] = buildRules[1] = null;
>+		// TODO: Fixed in Bug 331187 ; BuildManager#getRule is pessimistic when there's a mixed resource and null rule
>+		buildRules[0] = buildRules[1] = getWorkspace().getRoot();
>+		tb1.setStatus(TestBarrier.STATUS_START);
>+		tb2.setStatus(TestBarrier.STATUS_START);
>+		j.schedule();
>+		tb1.waitForStatus(TestBarrier.STATUS_DONE);
>+		tb2.waitForStatus(TestBarrier.STATUS_DONE);
>+	}
> }
>Index: src/org/eclipse/core/tests/internal/builders/TestBuilder.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/builders/TestBuilder.java,v
>retrieving revision 1.18
>diff -u -r1.18 TestBuilder.java
>--- src/org/eclipse/core/tests/internal/builders/TestBuilder.java	1 Feb 2011 11:37:48 -0000	1.18
>+++ src/org/eclipse/core/tests/internal/builders/TestBuilder.java	2 May 2011 14:51:48 -0000
>@@ -26,6 +26,8 @@
> 	 * A test specific call-back which can be ticked on #getRule(...) & #build(...)
> 	 */
> 	public static class BuilderRuleCallback {
>+		private IncrementalProjectBuilder builder;
>+
> 		public BuilderRuleCallback() {
> 		}
> 
>@@ -42,6 +44,10 @@
> 		public IProject[] build(int kind, Map<String, String> args, IProgressMonitor monitor) throws CoreException {
> 			return new IProject[0];
> 		}
>+
>+		public IResourceDelta getDelta(IProject project) {
>+			return builder.getDelta(project);
>+		}
> 	}
> 
> 	/**
>@@ -101,6 +107,7 @@
> 		logPluginLifecycleEvent(getBuildId());
> 		if (ruleCallBack == null)
> 			return new IProject[0];
>+		ruleCallBack.builder = this;
> 		return ruleCallBack.build(kind, args, monitor);
> 	}
> 
>@@ -111,6 +118,7 @@
> 	public ISchedulingRule getRule(int trigger, Map<String, String> args) {
> 		if (ruleCallBack == null)
> 			return super.getRule(trigger, args);
>+		ruleCallBack.builder = this;
> 		return ruleCallBack.getRule(name, this, trigger, args);
> 	}
>

Actions: View | Diff

Attachments on bug 343256: 193567 | 193622 | 193687 | 193763 | 193795 | 193849 | 193919 | 194499 | 194597