Bug 457687 - Dependency Injection should not catch Errors
According to the JavaDoc, an Error is "a subclass of Throwable that
indicates serious problems that a reasonable application should not try
to catch". Re-throw Errors that are raised by methods invoked by DI.
The underlying issue is that the Reflection API wraps Errors in an ITE.
It has been admitted in a JDK bug [1] that this is a bad practice but
cannot be changed for compatibility reasons. The recommended workaround
is to re-throw Errors wrapped in ITEs.
[1] http://bugs.java.com/bugdatabase/view_bug.do?bug_id=4386935
Change-Id: I109b64cc98b63e9852043fd232eb0664f3156ea4
Signed-off-by: Ralf Sternberg <rsternberg@eclipsesource.com>
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java
index 791a033..f4d9b9d 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/ConstructorRequestor.java
@@ -47,6 +47,11 @@
throw new InjectionException(e);
} catch (InvocationTargetException e) {
Throwable originalException = e.getCause();
+ // Errors such as ThreadDeath or OutOfMemoryError should not be trapped
+ // http://bugs.eclipse.org/bugs/show_bug.cgi?id=457687
+ if (originalException instanceof Error) {
+ throw (Error) originalException;
+ }
throw new InjectionException((originalException != null) ? originalException : e);
} finally {
if (!wasAccessible)
diff --git a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java
index 670ae42..6c29d21 100644
--- a/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java
+++ b/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/MethodRequestor.java
@@ -59,6 +59,11 @@
throw new InjectionException(e);
} catch (InvocationTargetException e) {
Throwable originalException = e.getCause();
+ // Errors such as ThreadDeath or OutOfMemoryError should not be trapped
+ // http://bugs.eclipse.org/bugs/show_bug.cgi?id=457687
+ if (originalException instanceof Error) {
+ throw (Error) originalException;
+ }
throw new InjectionException((originalException != null) ? originalException : e);
} finally {
if (!wasAccessible)
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/manual/InjectionErrorReportingTest.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/manual/InjectionErrorReportingTest.java
index f073132..1baf1f5 100644
--- a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/manual/InjectionErrorReportingTest.java
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/manual/InjectionErrorReportingTest.java
@@ -15,14 +15,13 @@
import javax.inject.Inject;
import javax.inject.Named;
-import junit.framework.TestCase;
-
import org.eclipse.e4.core.contexts.ContextInjectionFactory;
import org.eclipse.e4.core.contexts.EclipseContextFactory;
import org.eclipse.e4.core.contexts.IEclipseContext;
import org.eclipse.e4.core.di.InjectionException;
import org.eclipse.e4.core.di.annotations.Creatable;
-import org.junit.Ignore;
+
+import junit.framework.TestCase;
/**
* Manual test to observe error reporting. The JUnits in this
@@ -243,15 +242,17 @@
}
/**
- * Manual test to check error message for recursive object creation
+ * Manual test to check error message for recursive object creation Although
+ * bug 377343 disabled throwing InjectionExceptions on recursive creation,
+ * the fix for bug 457687 now exposes java.lang.Errors (such as
+ * StackOverflowError) rather than wrapping them in an InjectionException.
*/
- @Ignore("Exception on recursive creations removed with bug 377343")
public void testRecursionError() {
IEclipseContext context = EclipseContextFactory.create();
boolean exception = false;
try {
ContextInjectionFactory.make(InjectedRecursive.class, context);
- } catch (InjectionException e) {
+ } catch (StackOverflowError e) {
basicLog(e);
exception = true;
}
@@ -261,14 +262,14 @@
exception = false;
try {
ContextInjectionFactory.make(InjectedRecursive.class, context);
- } catch (InjectionException e) {
+ } catch (StackOverflowError e) {
basicLog(e);
exception = true;
}
assertFalse(exception);
}
- private void basicLog(InjectionException e) {
+ private void basicLog(Throwable e) {
e.printStackTrace(System.out);
}
}
