[115825] JSP Indexer can deadlock with AutoBuild job
diff --git a/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/JSPCorePlugin.java b/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/JSPCorePlugin.java
index 4497503..d0b1047 100644
--- a/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/JSPCorePlugin.java
+++ b/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/JSPCorePlugin.java
@@ -10,14 +10,10 @@
*******************************************************************************/
package org.eclipse.jst.jsp.core.internal;
-import org.eclipse.core.resources.IResourceChangeEvent;
-import org.eclipse.core.resources.IWorkspace;
-import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.Plugin;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jst.jsp.core.internal.contentmodel.TaglibController;
import org.eclipse.jst.jsp.core.internal.java.search.JSPIndexManager;
-import org.eclipse.jst.jsp.core.internal.java.search.JSPSearchSupport;
import org.eclipse.jst.jsp.core.internal.taglib.TaglibHelperManager;
import org.eclipse.jst.jsp.core.taglib.TaglibIndex;
import org.osgi.framework.BundleContext;
@@ -39,18 +35,13 @@
/**
* Returns the shared instance.
+ * @deprecated - will be removed. Currently used to get
+ * "model preferences", but there are other, better ways.
*/
public static JSPCorePlugin getDefault() {
return plugin;
}
- /**
- * Returns the workspace instance.
- */
- public static IWorkspace getWorkspace() {
- return ResourcesPlugin.getWorkspace();
- }
-
/* (non-Javadoc)
* @see org.eclipse.core.runtime.Plugin#start(org.osgi.framework.BundleContext)
*/
@@ -65,23 +56,16 @@
// listen for classpath changes
JavaCore.addElementChangedListener(TaglibHelperManager.getInstance());
- // add JSPIndexManager to keep JSP Index up to date
- // listening for IResourceChangeEvent.PRE_DELETE and IResourceChangeEvent.POST_CHANGE
- ResourcesPlugin.getWorkspace().addResourceChangeListener(JSPIndexManager.getInstance(), IResourceChangeEvent.POST_CHANGE);
- // https://w3.opensource.ibm.com/bugzilla/show_bug.cgi?id=5091
- // makes sure IndexManager is aware of our indexes
- JSPIndexManager.getInstance().saveIndexes();
+ JSPIndexManager.getInstance().initialize();
+
}
/* (non-Javadoc)
* @see org.eclipse.core.runtime.Plugin#stop(org.osgi.framework.BundleContext)
*/
public void stop(BundleContext context) throws Exception {
- // stop listening
- ResourcesPlugin.getWorkspace().removeResourceChangeListener(JSPIndexManager.getInstance());
- // stop any searching
- JSPSearchSupport.getInstance().setCanceled(true);
+
// stop any indexing
JSPIndexManager.getInstance().shutdown();
diff --git a/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/java/search/JSPIndexManager.java b/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/java/search/JSPIndexManager.java
index 13d74a1..827a520 100644
--- a/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/java/search/JSPIndexManager.java
+++ b/bundles/org.eclipse.jst.jsp.core/src/org/eclipse/jst/jsp/core/internal/java/search/JSPIndexManager.java
@@ -21,6 +21,7 @@
import org.eclipse.core.resources.IResourceChangeListener;
import org.eclipse.core.resources.IResourceDelta;
import org.eclipse.core.resources.IResourceDeltaVisitor;
+import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
@@ -47,9 +48,10 @@
*
* @author pavery
*/
-public class JSPIndexManager implements IResourceChangeListener {
+public class JSPIndexManager {
// for debugging
+ // TODO move this to Logger, as we have in SSE
static final boolean DEBUG;
static {
String value = Platform.getDebugOption("org.eclipse.jst.jsp.core/debug/jspindexmanager"); //$NON-NLS-1$
@@ -58,10 +60,11 @@
private static final String PKEY_INDEX_STATE = "jspIndexState"; //$NON-NLS-1$
- final IndexWorkspaceJob indexingJob = new IndexWorkspaceJob();
+ private IndexWorkspaceJob indexingJob = new IndexWorkspaceJob();
+ // TODO: consider enumeration for these int constants
// set to S_UPDATING once a resource change comes in
// set to S_STABLE if:
// - we know we aren't interested in the resource change
@@ -201,7 +204,8 @@
*/
private class ProcessFilesJob extends Job {
List fileList = null;
- private final int maximumToRemember = 50;
+ // keep track of how many files we've indexed
+ int lastFileCursor = 0;
ProcessFilesJob(String taskName) {
super(taskName);
@@ -215,18 +219,16 @@
if (DEBUG) {
System.out.println("JSPIndexManager queuing " + files.length + " files"); //$NON-NLS-2$ //$NON-NLS-1$
}
- schedule(20);
}
synchronized IFile[] getFiles() {
- IFile[] files = (IFile[]) fileList.toArray(new IFile[fileList.size()]);
- if (fileList.size() > maximumToRemember) {
- fileList = new ArrayList();
- }
- else {
- fileList.clear();
- }
- return files;
+ return (IFile[]) fileList.toArray(new IFile[fileList.size()]);
+ }
+
+ synchronized void clearFiles() {
+ fileList.clear();
+ lastFileCursor = 0;
+ //System.out.println("cleared files");
}
protected IStatus run(IProgressMonitor monitor) {
@@ -240,6 +242,7 @@
try {
IFile[] filesToBeProcessed = getFiles();
+
if (DEBUG) {
System.out.println("JSPIndexManager indexing " + filesToBeProcessed.length + " files"); //$NON-NLS-2$ //$NON-NLS-1$
}
@@ -249,21 +252,21 @@
String processingNFiles = ""; //$NON-NLS-1$
- for (int i = 0; i < filesToBeProcessed.length; i++) {
+ for (;lastFileCursor < filesToBeProcessed.length; lastFileCursor++) {
if (isCanceled(monitor) || frameworkIsShuttingDown()) {
setCanceledState();
return Status.CANCEL_STATUS;
}
try {
- ss.addJspFile(filesToBeProcessed[i]);
+ ss.addJspFile(filesToBeProcessed[lastFileCursor]);
// JSP Indexer processing n files
- processingNFiles = NLS.bind(JSPCoreMessages.JSPIndexManager_2, new String[]{Integer.toString((filesToBeProcessed.length - i))});
- monitor.subTask(processingNFiles + " - " + filesToBeProcessed[i].getName()); //$NON-NLS-1$
+ processingNFiles = NLS.bind(JSPCoreMessages.JSPIndexManager_2, new String[]{Integer.toString((filesToBeProcessed.length - lastFileCursor))});
+ monitor.subTask(processingNFiles + " - " + filesToBeProcessed[lastFileCursor].getName()); //$NON-NLS-1$
monitor.worked(1);
if (DEBUG) {
- System.out.println("JSPIndexManager Job added file: " + filesToBeProcessed[i].getName()); //$NON-NLS-1$
+ System.out.println("JSPIndexManager Job added file: " + filesToBeProcessed[lastFileCursor].getName()); //$NON-NLS-1$
}
}
catch (Exception e) {
@@ -281,7 +284,7 @@
// and only log a certain amt of the same one,
// otherwise skip it.
if (!frameworkIsShuttingDown()) {
- String filename = filesToBeProcessed[i] != null ? filesToBeProcessed[i].getFullPath().toString() : ""; //$NON-NLS-1$
+ String filename = filesToBeProcessed[lastFileCursor] != null ? filesToBeProcessed[lastFileCursor].getFullPath().toString() : ""; //$NON-NLS-1$
Logger.logException("JSPIndexer problem indexing:" + filename, e); //$NON-NLS-1$
}
}
@@ -293,6 +296,9 @@
monitor.done();
}
+ // successfully finished, clear files list
+ clearFiles();
+
long finish = System.currentTimeMillis();
long diff = finish - start;
if (DEBUG) {
@@ -320,18 +326,25 @@
}
return canceled;
}
+
}
// end class ProcessFilesJob
private static JSPIndexManager fSingleton = null;
+ private boolean initialized;
+ private boolean initializing = true;
+
+ private IndexJobCoordinator indexJobCoordinator;
+ private IResourceChangeListener jspResourceChangeListener;
+
private JSPResourceVisitor fVisitor = null;
private IContentType fContentTypeJSP = null;
static long fTotalTime = 0;
// Job for processing resource delta
- ProcessFilesJob processFilesJob = null;
+ private ProcessFilesJob processFilesJob = null;
private JSPIndexManager() {
processFilesJob = new ProcessFilesJob(JSPCoreMessages.JSPIndexManager_0);
@@ -353,75 +366,36 @@
return fSingleton;
}
- /**
- * @see org.eclipse.core.resources.IResourceChangeListener#resourceChanged(org.eclipse.core.resources.IResourceChangeEvent)
- */
- public void resourceChanged(IResourceChangeEvent event) {
+ public void initialize() {
- // ignore resource changes if already rebuilding
- if (getIndexState() == S_REBUILDING)
- return;
- // previously canceled, needs entire index rebuild
- if (getIndexState() == S_CANCELED) {
- rebuildIndex();
- return;
- }
+ JSPIndexManager singleInstance = getInstance();
- // set flag, so we know if a job is going to be started
- // and the state will eventually be set back to S_STABLE
- boolean beganProcess = false;
- setUpdatingState();
- IResourceDelta delta = event.getDelta();
- if (delta != null) {
- // only care about adds or changes right now...
- int kind = delta.getKind();
- boolean added = (kind & IResourceDelta.ADDED) == IResourceDelta.ADDED;
- boolean changed = (kind & IResourceDelta.CHANGED) == IResourceDelta.CHANGED;
- if (added || changed) {
+ if (!singleInstance.initialized) {
+ singleInstance.initialized = true;
- // only analyze the full (starting at root) delta hierarchy
- if (delta.getFullPath().toString().equals("/")) { //$NON-NLS-1$
- try {
- JSPResourceVisitor v = getVisitor();
- // clear from last run
- v.reset();
- // count files, possibly do this in a job too...
- // don't include PHANTOM resources
- delta.accept(v, false);
+ singleInstance.indexJobCoordinator = new IndexJobCoordinator();
+ singleInstance.jspResourceChangeListener = new JSPResourceChangeListener();
- // process files from this delta
- IFile[] files = v.getFiles();
- if (files.length > 0) {
- // processFiles(files);
- indexFiles(files);
- beganProcess = true;
- }
- }
- catch (CoreException e) {
- // need to set state here somehow, and reindex
- // otherwise index will be unreliable
- if (DEBUG)
- Logger.logException(e);
- }
- catch (Exception e) {
- // need to set state here somehow, and reindex
- // otherwise index will be unreliable
- if (DEBUG)
- Logger.logException(e);
- }
- }
- }
+ // added as JobChange listener so JSPIndexManager can be smarter
+ // about when it runs
+ Platform.getJobManager().addJobChangeListener(singleInstance.indexJobCoordinator);
+
+ // add JSPIndexManager to keep JSP Index up to date
+ // listening for IResourceChangeEvent.PRE_DELETE and
+ // IResourceChangeEvent.POST_CHANGE
+ ResourcesPlugin.getWorkspace().addResourceChangeListener(jspResourceChangeListener, IResourceChangeEvent.POST_CHANGE);
+
+ // https://w3.opensource.ibm.com/bugzilla/show_bug.cgi?id=5091
+ // makes sure IndexManager is aware of our indexes
+ saveIndexes();
+ singleInstance.initializing = false;
}
- // if we never kicked off process, job won't set back to stable
- // so we set it here
- if (!beganProcess) {
- setStableState();
- }
+
}
-
- public synchronized void setIndexState(int state) {
+
+ synchronized void setIndexState(int state) {
if (DEBUG) {
System.out.println("JSPIndexManager setting index state to: " + state2String(state)); //$NON-NLS-1$
}
@@ -450,41 +424,40 @@
return s;
}
- public int getIndexState() {
+ int getIndexState() {
return JSPCorePlugin.getDefault().getPluginPreferences().getInt(PKEY_INDEX_STATE);
}
- public void setUpdatingState() {
- if (getIndexState() != S_CANCELED)
- setIndexState(S_UPDATING);
+ void setUpdatingState() {
+ //if (getIndexState() != S_CANCELED)
+ setIndexState(S_UPDATING);
}
- public void setCanceledState() {
+ void setCanceledState() {
setIndexState(JSPIndexManager.S_CANCELED);
}
- // ca
- public void setStableState() {
- if (getIndexState() != S_CANCELED)
- setIndexState(S_STABLE);
+ void setStableState() {
+ //if (getIndexState() != S_CANCELED)
+ setIndexState(S_STABLE);
}
- public void setRebuildingState() {
+ void setRebuildingState() {
setIndexState(S_REBUILDING);
}
- public synchronized void rebuildIndexIfNeeded() {
+ synchronized void rebuildIndexIfNeeded() {
if (getIndexState() != S_STABLE) {
rebuildIndex();
}
}
- private void rebuildIndex() {
+ void rebuildIndex() {
if (DEBUG)
System.out.println("*** JSP Index unstable, requesting re-indexing"); //$NON-NLS-1$
- indexingJob.addJobChangeListener(new JobChangeAdapter() {
+ getIndexingJob().addJobChangeListener(new JobChangeAdapter() {
public void aboutToRun(IJobChangeEvent event) {
super.aboutToRun(event);
setRebuildingState();
@@ -493,10 +466,12 @@
public void done(IJobChangeEvent event) {
super.done(event);
setStableState();
- indexingJob.removeJobChangeListener(this);
+ getIndexingJob().removeJobChangeListener(this);
}
});
- indexingJob.schedule();
+ // we're about to reindex everything anyway
+ getProcessFilesJob().clearFiles();
+ getIndexingJob().schedule();
}
@@ -506,7 +481,7 @@
*
* @param files
*/
- public final void indexFiles(IFile[] files) {
+ final void indexFiles(IFile[] files) {
// don't use this rule
// https://w3.opensource.ibm.com/bugzilla/show_bug.cgi?id=4931
// processFiles.setRule(new IndexFileRule());
@@ -529,7 +504,7 @@
// https://w3.opensource.ibm.com/bugzilla/show_bug.cgi?id=5091
// makes sure IndexManager is aware of our indexes
- public void saveIndexes() {
+ void saveIndexes() {
IndexManager indexManager = JavaModelManager.getJavaModelManager().getIndexManager();
IPath jspModelWorkingLocation = JSPSearchSupport.getInstance().getModelJspPluginWorkingLocation();
@@ -580,6 +555,18 @@
public void shutdown() {
+
+ // stop listening
+ ResourcesPlugin.getWorkspace().removeResourceChangeListener(jspResourceChangeListener);
+
+
+ // stop any searching
+ JSPSearchSupport.getInstance().setCanceled(true);
+
+ // stop listening to jobs
+ Platform.getJobManager().removeJobChangeListener(indexJobCoordinator);
+
+
int maxwait = 5000;
if (processFilesJob != null) {
processFilesJob.cancel();
@@ -609,4 +596,133 @@
}
}
}
+
+ private class IndexJobCoordinator extends JobChangeAdapter {
+
+ public void aboutToRun(IJobChangeEvent event) {
+ Job jobToCoordinate = event.getJob();
+ if (isJobToAvoid(jobToCoordinate)) {
+ // job will be rescheduled when the job we
+ // are avoiding (eg. build) is done
+ getProcessFilesJob().cancel();
+ //System.out.println("cancel:" + jobToCoordinate.getName());
+ }
+ }
+
+ public void done(IJobChangeEvent event) {
+
+ Job jobToCoordinate = event.getJob();
+ if (isJobToAvoid(jobToCoordinate)) {
+ if (getProcessFilesJob().getFiles().length > 0) {
+ getProcessFilesJob().schedule(500);
+ //System.out.println("schedule:" + jobToCoordinate.getName());
+ }
+
+
+ }
+ }
+
+ private boolean isJobToAvoid(Job jobToCoordinate) {
+ boolean result = false;
+ if (jobToCoordinate.belongsTo(ResourcesPlugin.FAMILY_AUTO_BUILD) || jobToCoordinate.belongsTo(ResourcesPlugin.FAMILY_MANUAL_BUILD) || jobToCoordinate.belongsTo(ResourcesPlugin.FAMILY_AUTO_REFRESH)) {
+ result = true;
+ }
+ return result;
+
+ }
+
+ }
+
+ private class JSPResourceChangeListener implements IResourceChangeListener {
+
+
+ /**
+ * @see org.eclipse.core.resources.IResourceChangeListener#resourceChanged(org.eclipse.core.resources.IResourceChangeEvent)
+ */
+ public void resourceChanged(IResourceChangeEvent event) {
+
+ if (isInitializing())
+ return;
+
+ // ignore resource changes if already rebuilding
+ if (getIndexState() == S_REBUILDING)
+ return;
+ // previously canceled, needs entire index rebuild
+ if (getIndexState() == S_CANCELED) {
+ // rebuildIndex();
+ // just resume indexing
+ getProcessFilesJob().schedule(500);
+ //System.out.println("schedule: resource changed, previously canceled");
+ return;
+ }
+
+ // set flag, so we know if a job is going to be started
+ // and the state will eventually be set back to S_STABLE
+ boolean beganProcess = false;
+ setUpdatingState();
+
+ IResourceDelta delta = event.getDelta();
+ if (delta != null) {
+ // only care about adds or changes right now...
+ int kind = delta.getKind();
+ boolean added = (kind & IResourceDelta.ADDED) == IResourceDelta.ADDED;
+ boolean changed = (kind & IResourceDelta.CHANGED) == IResourceDelta.CHANGED;
+ if (added || changed) {
+
+ // only analyze the full (starting at root) delta
+ // hierarchy
+ if (delta.getFullPath().toString().equals("/")) { //$NON-NLS-1$
+ try {
+ JSPResourceVisitor v = getVisitor();
+ // clear from last run
+ v.reset();
+ // count files, possibly do this in a job too...
+ // don't include PHANTOM resources
+ delta.accept(v, false);
+
+ // process files from this delta
+ IFile[] files = v.getFiles();
+ if (files.length > 0) {
+ // processFiles(files);
+ indexFiles(files);
+ beganProcess = true;
+ }
+ }
+ catch (CoreException e) {
+ // need to set state here somehow, and reindex
+ // otherwise index will be unreliable
+ if (DEBUG)
+ Logger.logException(e);
+ }
+ catch (Exception e) {
+ // need to set state here somehow, and reindex
+ // otherwise index will be unreliable
+ if (DEBUG)
+ Logger.logException(e);
+ }
+ }
+ }
+
+ }
+ // if we never kicked off process, job won't set back to stable
+ // so we set it here
+ if (!beganProcess) {
+ setStableState();
+ }
+ }
+
+ }
+
+ IndexWorkspaceJob getIndexingJob() {
+ return indexingJob;
+ }
+
+ ProcessFilesJob getProcessFilesJob() {
+ return processFilesJob;
+ }
+
+ boolean isInitializing() {
+ return initializing;
+ }
+
}
\ No newline at end of file