Revert "Bug 558642: SafeRunner can return results"
This reverts commit 30247bf8acddfc1a8b89127aeb1caf66dad8ecf8.
Reason for revert: regressions in jface.text.tests and
core.tests.runtime.tests
Change-Id: Ibc3f827cb7dabb1f5c0b24b9d4636ee6782e3da3
Signed-off-by: Lars Vogel <Lars.Vogel@vogella.com>
diff --git a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java
index c209ab8..7edf55d 100644
--- a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java
+++ b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/SafeRunnerTest.java
@@ -13,10 +13,7 @@
*******************************************************************************/
package org.eclipse.equinox.common.tests;
-import org.eclipse.core.runtime.ISafeRunnable;
-import org.eclipse.core.runtime.ISafeRunnableWithResult;
-import org.eclipse.core.runtime.OperationCanceledException;
-import org.eclipse.core.runtime.SafeRunner;
+import org.eclipse.core.runtime.*;
import org.eclipse.core.tests.harness.CoreTest;
/**
@@ -25,36 +22,89 @@
public class SafeRunnerTest extends CoreTest {
/**
- * Ensures that cancellation exceptions are handled
+ * Ensures that cancelation exceptions are handled
*/
- public void testOperationCanceledExceptionAreHandled() {
+ public void testCancel() {
+ boolean caught = false;
try {
- SafeRunner.run(() -> {
- throw new OperationCanceledException();
+ SafeRunner.run(new ISafeRunnable() {
+ @Override
+ public void handleException(Throwable exception) {
+ }
+
+ @Override
+ public void run() throws Exception {
+ throw new OperationCanceledException();
+ }
});
} catch (OperationCanceledException e) {
- fail("OperationCanceledException unexpectedly caught.", e);
+ caught = true;
}
+ assertFalse("1.0", caught);
}
- public void testAssertionErrorIsCaught() {
- assertExceptionHandled(new AssertionError());
+ /**
+ * Tests that SafeRunner catches the expected exception types.
+ */
+ public void testCaughtExceptionTypes() {
+ Throwable[] failures = new Throwable[] {new AssertionError(), new LinkageError(), new RuntimeException()};
+ for (int i = 0; i < failures.length; i++) {
+ final Throwable[] handled = new Throwable[1];
+ final Throwable current = failures[i];
+ try {
+ SafeRunner.run(new ISafeRunnable() {
+ @Override
+ public void handleException(Throwable exception) {
+ handled[0] = exception;
+ }
+
+ @Override
+ public void run() throws Exception {
+ if (current instanceof Exception) {
+ throw (Exception) current;
+ } else if (current instanceof Error) {
+ throw (Error) current;
+ }
+ }
+ });
+ } catch (Throwable t) {
+ fail("1." + i, t);
+ }
+ assertEquals("2." + i, current, handled[0]);
+ }
+
}
- public void testLinkageErrorIsCaught() {
- assertExceptionHandled(new LinkageError());
- }
+ /**
+ * Tests that SafeRunner re-throws expected exception types.
+ */
+ public void testThrownExceptionTypes() {
+ //thrown exceptions
+ final Throwable[] thrown = new Throwable[] {new Error(), new OutOfMemoryError()};
+ for (int i = 0; i < thrown.length; i++) {
+ boolean caught = false;
+ final Throwable current = thrown[i];
+ try {
+ SafeRunner.run(new ISafeRunnable() {
+ @Override
+ public void handleException(Throwable exception) {
+ }
- public void testRuntimeExceptionIsCaught() {
- assertExceptionHandled(new RuntimeException());
- }
-
- public void testRethrowsError() {
- assertExceptionRethrown(new Error());
- }
-
- public void testRethrowsOutOfMemoryError() {
- assertExceptionRethrown(new OutOfMemoryError());
+ @Override
+ public void run() throws Exception {
+ if (current instanceof Exception) {
+ throw (Exception) current;
+ } else if (current instanceof Error) {
+ throw (Error) current;
+ }
+ }
+ });
+ } catch (Throwable t) {
+ assertEquals("1." + i, current, t);
+ caught = true;
+ }
+ assertTrue("2." + i, caught);
+ }
}
public void testNull() {
@@ -62,7 +112,7 @@
SafeRunner.run(null);
fail("1.0");
} catch (RuntimeException e) {
- // expected
+ //expected
}
}
@@ -70,7 +120,7 @@
* Ensures that exceptions are propagated when the safe runner re-throws it
*/
public void testRethrow() {
- IllegalArgumentException caughtException = null;
+ boolean caught = false;
try {
SafeRunner.run(new ISafeRunnable() {
@Override
@@ -86,64 +136,9 @@
}
});
} catch (IllegalArgumentException e) {
- caughtException = e;
+ caught = true;
}
- assertNotNull("Cathed exception expected.", caughtException);
+ assertTrue("1.0", caught);
}
-
- public void testWithResult() {
- assertEquals("TestRun", SafeRunner.run(() -> "TestRun"));
- }
-
- public void testWithResultReturnsNullOnException() {
- ISafeRunnableWithResult<String> code = () -> {
- throw new IllegalArgumentException();
- };
- assertNull(SafeRunner.run(code));
- }
-
- private void assertExceptionRethrown(Throwable current) {
- Throwable caughtException = null;
- try {
- SafeRunner.run(new ISafeRunnable() {
-
- @Override
- public void run() throws Exception {
- if (current instanceof Exception) {
- throw (Exception) current;
- } else if (current instanceof Error) {
- throw (Error) current;
- }
- }
- });
- } catch (Throwable t) {
- caughtException = t;
- }
- assertEquals("Unexpected exception.", current, caughtException);
- }
-
- private void assertExceptionHandled(Throwable throwable) {
- final Throwable[] handled = new Throwable[1];
- try {
- SafeRunner.run(new ISafeRunnable() {
- @Override
- public void handleException(Throwable exception) {
- handled[0] = exception;
- }
-
- @Override
- public void run() throws Exception {
- if (throwable instanceof Exception) {
- throw (Exception) throwable;
- } else if (throwable instanceof Error) {
- throw (Error) throwable;
- }
- }
- });
- } catch (Throwable t) {
- fail("Exception unexpectedly caught.", t);
- }
- assertEquals("Unexpected exception.", throwable, handled[0]);
- }
-}
\ No newline at end of file
+}
diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java
deleted file mode 100644
index 4f98a94..0000000
--- a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ISafeRunnableWithResult.java
+++ /dev/null
@@ -1,35 +0,0 @@
-package org.eclipse.core.runtime;
-
-/**
- * Safe runnables represent blocks of code and associated exception
- * handlers. They are typically used when a plug-in needs to call some
- * untrusted code (e.g., code contributed by another plug-in via an
- * extension). In contradiction to {@link ISafeRunnable} this runnable is able to return a result.
- * <p>
- * This interface can be used without OSGi running.
- * </p><p>
- * Clients may implement this interface.
- * </p>
- * @param <T> the type of the result
- * @see SafeRunner#run(ISafeRunnableWithResult)
- *
- * @since 3.11
- */
-@FunctionalInterface
-public interface ISafeRunnableWithResult<T> extends ISafeRunnable {
- @Override
- default void run() throws Exception {
- runWithResult();
- }
-
- /**
- * Runs this runnable and returns the result. Any exceptions thrown from this method will
- * be logged by the caller and passed to this runnable's
- * {@link #handleException} method.
- * @return the result
- *
- * @exception Exception if a problem occurred while running this method
- * @see SafeRunner#run(ISafeRunnable)
- */
- public T runWithResult() throws Exception;
-}
\ No newline at end of file
diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java
index 3a08701..3d6861f 100644
--- a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java
+++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/SafeRunner.java
@@ -28,9 +28,9 @@
public final class SafeRunner {
/**
- * Runs the given runnable in a protected mode. Exceptions
+ * Runs the given runnable in a protected mode. Exceptions
* thrown in the runnable are logged and passed to the runnable's
- * exception handler. Such exceptions are not rethrown by this method.
+ * exception handler. Such exceptions are not rethrown by this method.
* <p>
* In addition to catching all {@link Exception} types, this method also catches certain {@link Error}
* types that typically result from programming errors in the code being executed.
@@ -40,81 +40,37 @@
* @param code the runnable to run
*/
public static void run(ISafeRunnable code) {
- run(new ISafeRunnableWithResult<Void>() {
-
- @Override
- public Void runWithResult() throws Exception {
- code.run();
- return null;
- }
-
- @Override
- public void handleException(Throwable exception) {
- code.handleException(exception);
- }
- });
- }
-
- /**
- * Runs the given runnable in a protected mode and returns the result given by the runnable. Exceptions
- * thrown in the runnable are logged and passed to the runnable's
- * exception handler. Such exceptions are not rethrown by this method, instead null is returned.
- * <p>
- * In addition to catching all {@link Exception} types, this method also catches certain {@link Error}
- * types that typically result from programming errors in the code being executed.
- * Severe errors that are not generally safe to catch are not caught by this method.
- * </p>
- * @param <T> the result type
- *
- * @param code the runnable to run
- * @return the result
- *
- * @since 3.11
- */
- public static <T> T run(ISafeRunnableWithResult<T> code) {
Assert.isNotNull(code);
try {
- return code.runWithResult();
+ code.run();
} catch (Exception | LinkageError | AssertionError e) {
handleException(code, e);
- return null;
}
}
- private static void handleException(ISafeRunnable code, Throwable exception) {
- if (!(exception instanceof OperationCanceledException)) {
- String pluginId = getBundleIdOfSafeRunnable(code);
- IStatus status = convertToStatus(exception, pluginId);
- makeSureUserSeesException(exception, status);
+ private static void handleException(ISafeRunnable code, Throwable e) {
+ if (!(e instanceof OperationCanceledException)) {
+ // try to obtain the correct plug-in id for the bundle providing the safe runnable
+ Activator activator = Activator.getDefault();
+ String pluginId = null;
+ if (activator != null)
+ pluginId = activator.getBundleId(code);
+ if (pluginId == null)
+ pluginId = IRuntimeConstants.PI_COMMON;
+ String message = NLS.bind(CommonMessages.meta_pluginProblems, pluginId);
+ IStatus status;
+ if (e instanceof CoreException) {
+ status = new MultiStatus(pluginId, IRuntimeConstants.PLUGIN_ERROR, message, e);
+ ((MultiStatus) status).merge(((CoreException) e).getStatus());
+ } else {
+ status = new Status(IStatus.ERROR, pluginId, IRuntimeConstants.PLUGIN_ERROR, message, e);
+ }
+ // Make sure user sees the exception: if the log is empty, log the exceptions on stderr
+ if (!RuntimeLog.isEmpty())
+ RuntimeLog.log(status);
+ else
+ e.printStackTrace();
}
- code.handleException(exception);
+ code.handleException(e);
}
-
- private static void makeSureUserSeesException(Throwable exception, IStatus status) {
- if (RuntimeLog.isEmpty()) {
- exception.printStackTrace();
- } else {
- RuntimeLog.log(status);
- }
- }
-
- private static String getBundleIdOfSafeRunnable(ISafeRunnable code) {
- Activator activator = Activator.getDefault();
- String pluginId = null;
- if (activator != null)
- pluginId = activator.getBundleId(code);
- if (pluginId == null)
- return IRuntimeConstants.PI_COMMON;
- return pluginId;
- }
-
- private static IStatus convertToStatus(Throwable exception, String pluginId) {
- String message = NLS.bind(CommonMessages.meta_pluginProblems, pluginId);
- if (exception instanceof CoreException) {
- MultiStatus status = new MultiStatus(pluginId, IRuntimeConstants.PLUGIN_ERROR, message, exception);
- status.merge(((CoreException) exception).getStatus());
- return status;
- }
- return new Status(IStatus.ERROR, pluginId, IRuntimeConstants.PLUGIN_ERROR, message, exception);
- }
-}
\ No newline at end of file
+}
