Attachment 227414 Details for Bug 265324 – Patch v.0.1

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

[patch] Patch v.0.1

patch265324.txt (text/plain), 8.71 KB, created by Szymon Ptaszkiewicz

on 2013-02-21 13:39:51 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Szymon Ptaszkiewicz

Created: 2013-02-21 13:39:51 EST

Size: 8.71 KB

patch

obsolete

>diff --git a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/DeadlockDetector.java b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/DeadlockDetector.java
>index b52df12..1adfb67 100644
>--- a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/DeadlockDetector.java
>+++ b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/DeadlockDetector.java
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2003, 2012 IBM Corporation and others.
>+ * Copyright (c) 2003, 2013 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -184,17 +184,25 @@
> 	/**
> 	 * Returns all the locks owned by the given thread
> 	 */
>-	private Object[] getOwnedLocks(Thread current) {
>+	ISchedulingRule[] getOwnedLocks(Thread thread) {
>+		return getOwnedLocks(thread, false);
>+	}
>+
>+	/**
>+	 * Returns all the locks owned by the given thread
>+	 */
>+	private ISchedulingRule[] getOwnedLocks(Thread current, boolean deadlocked) {
> 		ArrayList ownedLocks = new ArrayList(1);
> 		int index = indexOf(current, false);
>-
>-		for (int j = 0; j < graph[index].length; j++) {
>-			if (graph[index][j] > NO_STATE)
>-				ownedLocks.add(locks.get(j));
>+		if (index >= 0) {
>+			for (int j = 0; j < graph[index].length; j++) {
>+				if (graph[index][j] > NO_STATE)
>+					ownedLocks.add(locks.get(j));
>+			}
> 		}
>-		if (ownedLocks.size() == 0)
>+		if (deadlocked && ownedLocks.size() == 0)
> 			Assert.isLegal(false, "A thread with no locks is part of a deadlock."); //$NON-NLS-1$
>-		return ownedLocks.toArray();
>+		return (ISchedulingRule[]) ownedLocks.toArray(new ISchedulingRule[ownedLocks.size()]);
> 	}
> 
> 	/**
>@@ -599,7 +607,7 @@
> 		MultiStatus main = new MultiStatus(JobManager.PI_JOBS, JobManager.PLUGIN_ERROR, msg, new IllegalStateException());
> 		Thread[] threads = deadlock.getThreads();
> 		for (int i = 0; i < threads.length; i++) {
>-			Object[] ownedLocks = getOwnedLocks(threads[i]);
>+			Object[] ownedLocks = getOwnedLocks(threads[i], true);
> 			Object waitLock = getWaitingLock(threads[i]);
> 			StringBuffer buf = new StringBuffer("Thread "); //$NON-NLS-1$
> 			buf.append(threads[i].getName());
>diff --git a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/LockManager.java b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/LockManager.java
>index 0a8fa2f..6028a60 100644
>--- a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/LockManager.java
>+++ b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/LockManager.java
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- *  Copyright (c) 2003, 2012 IBM Corporation and others.
>+ *  Copyright (c) 2003, 2013 IBM Corporation and others.
>  *  All rights reserved. This program and the accompanying materials
>  *  are made available under the terms of the Eclipse Public License v1.0
>  *  which accompanies this distribution, and is available at
>@@ -177,6 +177,20 @@
> 		}
> 	}
> 
>+	ISchedulingRule[] getOwnedLocks(Thread thread) {
>+		DeadlockDetector tempLocks = locks;
>+		if (tempLocks == null)
>+			return null;
>+		try {
>+			synchronized (tempLocks) {
>+				return tempLocks.getOwnedLocks(thread);
>+			}
>+		} catch (Exception e) {
>+			handleInternalError(e);
>+		}
>+		return null;
>+	}
>+
> 	/**
> 	 * Handles exceptions that occur while calling third party code from within the
> 	 * LockManager. This is essentially an in-lined version of Platform.run(ISafeRunnable)
>diff --git a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/WorkerPool.java b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/WorkerPool.java
>index f876633..04a873b 100644
>--- a/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/WorkerPool.java
>+++ b/bundles/org.eclipse.core.jobs/src/org/eclipse/core/internal/jobs/WorkerPool.java
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- *  Copyright (c) 2003, 2012 IBM Corporation and others.
>+ *  Copyright (c) 2003, 2013 IBM Corporation and others.
>  *  All rights reserved. This program and the accompanying materials
>  *  are made available under the terms of the Eclipse Public License v1.0
>  *  which accompanies this distribution, and is available at
>@@ -10,9 +10,9 @@
>  *******************************************************************************/
> package org.eclipse.core.internal.jobs;
> 
>-import org.eclipse.core.runtime.Assert;
>-import org.eclipse.core.runtime.IStatus;
>-import org.eclipse.core.runtime.jobs.Job;
>+import org.eclipse.core.internal.runtime.RuntimeLog;
>+import org.eclipse.core.runtime.*;
>+import org.eclipse.core.runtime.jobs.*;
> 
> /**
>  * Maintains a pool of worker threads. Threads are constructed lazily as
>@@ -81,6 +81,37 @@
> 		threads[numThreads++] = worker;
> 	}
> 
>+	private void cleanWorker(InternalJob job) {
>+		//there should be no locks owned by the current thread at this moment, release if there are any left
>+		ISchedulingRule[] locks = manager.getLockManager().getOwnedLocks(Thread.currentThread());
>+		if (locks != null && locks.length > 0) {
>+			String msg = "Worker thread ended job: " + job + ", but still holds locks: ["; //$NON-NLS-1$ //$NON-NLS-2$
>+			for (int i = 0; i < locks.length; i++) {
>+				//it is allowed to call ILock#release here because it is called in the same thread in
>+				//which job was running but failed to release the lock in its Job#run method
>+				//it is still a bug in client code that they failed to release the lock, but to avoid
>+				//a deadlock we release it for them
>+				if (locks[i] instanceof ILock) {
>+					ILock lock = (ILock) locks[i];
>+					int depth = lock.getDepth();
>+					for (int j = 0; j < depth; j++)
>+						lock.release();
>+				}
>+				msg += locks[i].toString();
>+				if (i < locks.length - 1)
>+					msg += ","; //$NON-NLS-1$
>+			}
>+			msg += "]"; //$NON-NLS-1$
>+			IStatus error = new Status(IStatus.ERROR, JobManager.PI_JOBS, 1, msg, null);
>+			try {
>+				RuntimeLog.log(error);
>+			} catch (RuntimeException e) {
>+				//failed to log, so print to console instead
>+				System.err.println(error.getMessage());
>+			}
>+		}
>+	}
>+
> 	private synchronized void decrementBusyThreads() {
> 		//impossible to have less than zero busy threads
> 		if (--busyThreads < 0) {
>@@ -106,6 +137,7 @@
> 			//ensure this thread no longer owns any scheduling rules
> 			manager.implicitJobs.endJob(job);
> 		} finally {
>+			cleanWorker(job);
> 			decrementBusyThreads();
> 		}
> 	}
>diff --git a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java
>index bacbcc9..6cf5fc8 100644
>--- a/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java
>+++ b/tests/org.eclipse.core.tests.runtime/src/org/eclipse/core/tests/runtime/jobs/IJobManagerTest.java
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2003, 2012 IBM Corporation and others.
>+ * Copyright (c) 2003, 2013 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>@@ -137,6 +137,45 @@
> 		//		manager.startup();
> 	}
> 
>+	/**
>+	 * Tests running a job that acquires a lock but never releases it
>+	 */
>+	public void testAcquireLockNoRelease() {
>+		final ILock lock1 = manager.newLock();
>+		final ILock lock2 = manager.newLock();
>+		Job job = new Job("testAcquireLockNoRelease") {
>+			protected IStatus run(IProgressMonitor monitor) {
>+				monitor.beginTask(getName(), 1);
>+				try {
>+					lock1.acquire();
>+					lock1.acquire();
>+					lock1.acquire();
>+					lock2.acquire();
>+					monitor.worked(1);
>+				} finally {
>+					monitor.done();
>+				}
>+				return Status.OK_STATUS;
>+			}
>+		};
>+		job.schedule();
>+		try {
>+			job.join();
>+		} catch (InterruptedException e) {
>+			fail("1.0", e);
>+		}
>+		//another thread should be able to acquire the lock now
>+		try {
>+			assertTrue("2.1", lock1.acquire(10000));
>+			assertTrue("2.2", lock2.acquire(10000));
>+		} catch (InterruptedException e) {
>+			fail("3.0", e);
>+		} finally {
>+			lock1.release();
>+			lock2.release();
>+		}
>+	}
>+
> 	public void testBeginInvalidNestedRules() {
> 		final ISchedulingRule root = new PathRule("/");
> 		final ISchedulingRule invalid = new ISchedulingRule() {

Actions: View | Diff

Attachments on bug 265324: 126039 | 227414