Attachment 163842 Details for Bug 308120 – Patch that uses ILocks to avoid deadlock

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Patch that uses ILocks to avoid deadlock

sse.yield2.patch (text/plain), 6.45 KB, created by Min Idzelis

on 2010-04-05 17:09:55 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Min Idzelis

Created: 2010-04-05 17:09:55 EDT

Size: 6.45 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.wst.sse.core
>Index: src/org/eclipse/wst/sse/core/internal/model/ModelManagerImpl.java
>===================================================================
>RCS file: /cvsroot/webtools/sourceediting/plugins/org.eclipse.wst.sse.core/src/org/eclipse/wst/sse/core/internal/model/ModelManagerImpl.java,v
>retrieving revision 1.41
>diff -u -r1.41 ModelManagerImpl.java
>--- src/org/eclipse/wst/sse/core/internal/model/ModelManagerImpl.java	29 Mar 2010 21:56:39 -0000	1.41
>+++ src/org/eclipse/wst/sse/core/internal/model/ModelManagerImpl.java	5 Apr 2010 21:09:16 -0000
>@@ -41,6 +41,7 @@
> import org.eclipse.core.runtime.IPath;
> import org.eclipse.core.runtime.IStatus;
> import org.eclipse.core.runtime.NullProgressMonitor;
>+import org.eclipse.core.runtime.OperationCanceledException;
> import org.eclipse.core.runtime.Path;
> import org.eclipse.core.runtime.Platform;
> import org.eclipse.core.runtime.QualifiedName;
>@@ -103,75 +104,73 @@
> 		}
> 	}
> 
>-	/**
>-	 * A Data class to track our shared objects
>-	 */
>-	 static class SharedObject {
>+	class SharedObject {
> 		int referenceCountForEdit;
> 		int referenceCountForRead;
>-		IStructuredModel theSharedModel;
>+		volatile IStructuredModel theSharedModel;
>+		final ILock LOAD_LOCK = Job.getJobManager().newLock();
> 		volatile boolean initializing = true;
> 		volatile boolean doWait = true;
>-		
>-		SharedObject(IStructuredModel sharedModel) {
>-			theSharedModel = sharedModel;
>-			referenceCountForRead = 0;
>-			referenceCountForEdit = 0;
>+		// The field 'id' is only meant for debug
>+		volatile String id;
>+
>+		SharedObject(String id) {
>+			this.id=id;
>+			// be aware, this lock will leak and cause the deadlock detector to be horrible if we never release it
>+			LOAD_LOCK.acquire();
> 		}
>-		
>+
> 		/**
>-		 * Waits until this shared object has been attempted to be loaded. 
>-		 * The load is "attempted" because not all loads result in a model. 
>-		 * However, upon leaving this method, theShareModel variable
>-		 * is up-to-date.
>+		 * Waits until this shared object has been attempted to be loaded. The
>+		 * load is "attempted" because not all loads result in a model. However,
>+		 * upon leaving this method, theShareModel variable is up-to-date.
> 		 */
> 		public void waitForLoadAttempt() {
>+			Job current = Job.getJobManager().currentJob();
> 			boolean interrupted = false;
> 			try {
>-				// if we have a rule, then use a polling system with
>-				// short-circuit, otherwise use wait/notify
>-				if (Job.getJobManager().currentRule() != null) {
>-					while (initializing) {
>-						Job.getJobManager().currentJob().yieldRule(null);
>-						synchronized (this) {
>-							if (initializing) {
>-								try {
>-									wait(WAIT_INTERVAL_MS);
>-								}
>-								catch (InterruptedException e) {
>-									interrupted = true;
>-								}
>-							}
>-						}
>+				while (initializing) {
>+					if (current!=null) {
>+						current.yieldRule(null);
> 					}
>-				}
>-				else {
>-					synchronized (this) {
>-						while (initializing) {
>-							try {
>-								wait();
>-							}
>-							catch (InterruptedException e) {
>-								interrupted = true;
>-							}
>-						}
>+					try {
>+						loop();
>+					} catch (InterruptedException e) {
>+						interrupted=true;
> 					}
> 				}
> 			}
> 			finally {
>-				if (interrupted)
>+				if (interrupted) {
> 					Thread.currentThread().interrupt();
>+				}
> 			}
> 		}
>-		
>+
>+		private void loop() throws InterruptedException {	
>+			if (initializing) {
>+				if (LOAD_LOCK.acquire(WAIT_INTERVAL_MS)) {
>+					// if we got the lock, but initializing is still not true the deadlock detector gave us
>+					// the lock and caused reentrancy into this critical section. This is invalid and the 
>+					// sign of a cyclical load attempt. In this case, we through an 
>+					// OperationCanceledException in lew of entering a spin-loop. 
>+					if (initializing) {
>+						LOAD_LOCK.release();
>+						throw new OperationCanceledException("Aborted cyclic load attempt for model with id: "+ id);
>+					} else {
>+						LOAD_LOCK.release();
>+					}
>+				}
>+			}
>+		}
>+
> 		/**
>-		 * Flags this model as loaded. All waiting methods on 
>-		 * {@link #waitForLoadAttempt()} will proceed after this 
>-		 * method returns. 
>+		 * Flags this model as loaded. All waiting methods on
>+		 * {@link #waitForLoadAttempt()} will proceed after this method returns.
> 		 */
>-		public synchronized void setLoaded() {
>+		public void setLoaded() {
> 			initializing = false;
>-			notifyAll();
>+			LOAD_LOCK.release();
> 		}
> 	}
> 
>@@ -239,7 +238,7 @@
> 			SharedObject testObject = (SharedObject) fManagedObjects.get(id);
> 			if (testObject==null) {
> 				// null means it's been disposed, we need to do the work to reload it.
>-				sharedObject = new SharedObject(null);
>+				sharedObject = new SharedObject(id);
> 				fManagedObjects.put(id, sharedObject);
> 				SYNC.release();
> 				_doCommonCreateModel(file, id, handler, resolver, rwType, encodingRule,
>@@ -350,7 +349,7 @@
> 			SharedObject testObject = (SharedObject) fManagedObjects.get(id);
> 			if (testObject==null) {
> 				// it was removed ,so lets create it
>-				sharedObject = new SharedObject(null);
>+				sharedObject = new SharedObject(id);
> 				fManagedObjects.put(id, sharedObject);
> 				SYNC.release();
> 				_doCommonCreateModel(inputStream, id, handler, resolver, rwType,
>@@ -493,7 +492,7 @@
> 				SharedObject testObject = (SharedObject) fManagedObjects.get(id);
> 				if (testObject==null) {
> 					// it was removed ,so lets create it
>-					sharedObject = new SharedObject(null);
>+					sharedObject = new SharedObject(id);
> 					fManagedObjects.put(id, sharedObject);
> 					
> 					SYNC.release();
>@@ -569,7 +568,7 @@
> 				throw new ResourceInUse();
> 			}
> 			
>-			sharedObject = new SharedObject(null);
>+			sharedObject = new SharedObject(id);
> 			fManagedObjects.put(id, sharedObject);
> 			
> 		} finally {
>@@ -614,7 +613,7 @@
> 			SYNC.acquire();
> 			SharedObject testObject = (SharedObject) fManagedObjects.get(id);
> 			if (testObject==null) {
>-				sharedObject = new SharedObject(null);
>+				sharedObject = new SharedObject(id);
> 				fManagedObjects.put(id, sharedObject);
> 				SYNC.release();
> 				synchronized(sharedObject) {
>@@ -832,7 +831,7 @@
> 			if (sharedObject != null) {
> 				throw new ResourceInUse();
> 			}
>-			sharedObject = new SharedObject(null);
>+			sharedObject = new SharedObject(newId);
> 			fManagedObjects.put(newId,sharedObject);
> 		} finally {
> 			SYNC.release();

Actions: View | Diff

Attachments on bug 308120: 163842 | 165769