29

I have recently discovered and blogged about the fact that it is possible to sneak a checked exception through the javac compiler and throw it where it mustn't be thrown. This compiles and runs in Java 6 and 7, throwing a SQLException without throws or catch clause:

public class Test {

    // No throws clause here
    public static void main(String[] args) {
        doThrow(new SQLException());
    }

    static void doThrow(Exception e) {
        Test.<RuntimeException> doThrow0(e);
    }

    static <E extends Exception> void doThrow0(Exception e) throws E {
        throw (E) e;
    }
}

The generated bytecode indicates that the JVM doesn't really care about checked / unchecked exceptions:

// Method descriptor #22 (Ljava/lang/Exception;)V
// Stack: 1, Locals: 1
static void doThrow(java.lang.Exception e);
  0  aload_0 [e]
  1  invokestatic Test.doThrow0(java.lang.Exception) : void [25]
  4  return
    Line numbers:
      [pc: 0, line: 11]
      [pc: 4, line: 12]
    Local variable table:
      [pc: 0, pc: 5] local: e index: 0 type: java.lang.Exception

// Method descriptor #22 (Ljava/lang/Exception;)V
// Signature: <E:Ljava/lang/Exception;>(Ljava/lang/Exception;)V^TE;
// Stack: 1, Locals: 1
static void doThrow0(java.lang.Exception e) throws java.lang.Exception;
  0  aload_0 [e]
  1  athrow
    Line numbers:
      [pc: 0, line: 16]
    Local variable table:
      [pc: 0, pc: 2] local: e index: 0 type: java.lang.Exception

The JVM accepting this is one thing. But I have some doubts whether Java-the-language should. Which parts of the JLS justify this behaviour? Is it a bug? Or a well-hidden "feature" of the Java language?

My feelings are:

  • doThrow0()'s <E> is bound to RuntimeException in doThrow(). Hence, no throws clause along the lines of JLS §11.2 is needed in doThrow().
  • RuntimeException is assignment-compatible with Exception, hence no cast (which would result in a ClassCastException) is generated by the compiler.
Lukas Eder
  • 211,314
  • 129
  • 689
  • 1,509
  • Because `RuntimeException` extends Exception this is possible. But it is allowed. – Amit Deshpande Sep 25 '12 at 10:07
  • 1
    The JVM couldn't care less, that is for sure. It's only a compile-time validation. I like this, maybe I can adapt it to finally be able to avoid all those idiotic try-catch e-throw new RuntimeException(e)! – Marko Topolnik Sep 25 '12 at 10:07
  • Dude, this is a nice way to get both reputations on SO _and_ click-through on your blog ;) – Saintali Sep 25 '12 at 11:40
  • I actually liked it. It gave the question more context. Spam is such a strong word for that. – Saintali Sep 25 '12 at 14:04

5 Answers5

9

All this amounts to exploiting the loophole that an unchecked cast to a generic type is not a compiler error. Your code is explicitly made type-unsafe if it contains such an expression. And since the checking of checked exceptions is strictly a compile-time procedure, nothing will break in the runtime.

The answer from the authors of Generics would most probably be along the lines of "If you are using unchecked casts, it's your problem".

I see something quite positive in your discovery—a break out of the stronghold of checked exceptions. Unfortunately, this can't turn existing checked-exception-poisoned APIs into something more pleasant to use.

How this can help

At a typical layered application project of mine there will be a lot of boilerplate like this:

try {
  ... business logic stuff ...
} 
catch (RuntimeException e) { throw e; } 
catch (Exception e) { throw new RuntimeException(e); }

Why do I do it? Simple: there are no business-value exceptions to catch; any exception is a symptom of a runtime error. The exception must be propagated up the call stack towards the global exception barrier. With Lukas' fine contribution I can now write

try {
  ... business logic stuff ... 
} catch (Exception e) { throwUnchecked(e); }

This may not seem like much, but the benefits accumulate after repeating it 100 times throughout the project.

Disclaimer

In my projects there is high discipline regarding exceptions so this is a nice fit for them. This kind of trickery is not something to adopt as a general coding principle. Wrapping the exception is still the only safe option in many cases.

Community
  • 1
  • 1
Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436
  • 2
    *"checked-exception-poisoned APIs"* ;-) – assylias Sep 25 '12 at 10:52
  • 3
    That can thoroughly break callers who expect checked exceptions to be declared. Imagine a try block containing multiple invocations. One of the methods called declares `throws FooException`, another does not but throws it anyways. A catch block handling `FooException` might not expect the state the program is in when that comes to pass. – Ben Schulz Sep 25 '12 at 10:53
  • @Ben I agree, this is definitely not to be seen as a general recommendation. First of all, it helps only when you are authoring code that throws the exception, and in that case you should just choose an unchecked exception to throw. – Marko Topolnik Sep 25 '12 at 10:55
  • @MarkoTopolnik: On a side-note, the stacktrace won't improve, as the location of `new SQLException()` remains the same. The location of the actual `throw` statement is irrelevant to the stack trace. – Lukas Eder Sep 25 '12 at 11:21
  • Excellent point, Lucas. I oversaw it. This renders my code useless to the power of two! – Marko Topolnik Sep 25 '12 at 11:25
  • @MarkoTopolnik: About your disclaimer, I can see 1-2 use cases in framework logic. Instead of wrapping exceptions in `RuntimeException`, obfuscating their stack trace, you can just bypass all sorts of layers. In remote cases, this can be useful... – Lukas Eder Sep 25 '12 at 11:40
  • @LukasEder So you suggest `try-catch e-un.throwun(e)`? Yes, a good idea. I'll change my text. – Marko Topolnik Sep 25 '12 at 11:44
  • @Saintali I have given very careful justification for my code. You had no reason to downvote. NB I have many thousands of lines of code in production, satisfying thousands of users. So please, point out the catastrophe! – Marko Topolnik Sep 25 '12 at 13:04
  • @LukasEder Now you can see why I put the disclaimer in--removed it and presto---a downvote! – Marko Topolnik Sep 25 '12 at 13:07
  • @MarkoTopolnik: Man that sucks. But it wasn't Saintali... And I don't think it's catastrophic either. I'm sure spring does this all the time... – Lukas Eder Sep 25 '12 at 13:26
  • @LukasEder Every modern framework wraps all checked exceptions and that is considered a godsend, but it doesn't make code throw undeclared checked exceptions, so I assume that was Saintali's complaint. BTW how do you figure it's not his downvote, the timestamps of his comment and the DV coincide very precisely? – Marko Topolnik Sep 25 '12 at 13:30
  • @MarkoTopolnik This technique is justifiable and useful, only where you are calling a callback given to you by the caller. The so called "exception transparency" idiom..... I suspect spring uses this technique only for this purpose. Things like `callInTransaction(Callback)`, `callAsFoo(Callback)`, etc. come to mind. – Saintali Sep 25 '12 at 14:01
  • @Saintali But note my use case: I have both the caller and the callee side under my control, which is the typical situation in a layered app. My service layer doesn't normally throw any business-level exceptions, and if it did, that would receive special handling. **Any exception**, be it checked or unchecked, is a sign of a runtime error. The distinction is completely artificial, so I try my best to undo it. Call it the "All exceptions are created equal" movement :) – Marko Topolnik Sep 25 '12 at 14:09
  • I like that disclaimer. But even in that case you should take extra care not to use that style in a library, and also never hand your code to libraries to call it. I rescinded my down vote, but I think you should be more explicit about those assumptions in your disclaimer. – Saintali Sep 25 '12 at 14:11
  • 1
    @Saintali I would **absolutely, positively, never, ever** use such tricks in a public library! But let's land on the Earth and acknowledge that nobody ever writes public libraries, except for a select few, and even for them it's 10% of their code tops. I can write something to that effect in the disclaimer, but my use case specification already covers the ground pretty well, I'd say – Marko Topolnik Sep 25 '12 at 14:18
2

Well, that's one of multiple ways to raise a check exception as if it were unchecked. Class.newInstance() is another, Thread.stop(Trowable) a deprecated one.

The only way for the JLS not to accept this behavior is if the runtime (JVM) will enforce it.

As to were it is specified: It isn't. Checked and unchecked exceptions behave the same. Checked exceptions simply require a catch block or a throws clause.

Edit: Based on the discussion in the comments, a list-based example exposing the root cause: Erasure

public class Main {
    public static void main(String[] args) {
        List<Exception> myCheckedExceptions = new ArrayList<Exception>();
        myCheckedExceptions.add(new IOException());

        // here we're tricking the compiler
        @SuppressWarnings("unchecked")
        List<RuntimeException> myUncheckedExceptions = (List<RuntimeException>) (Object) myCheckedExceptions;

        // here we aren't any more, at least not beyond the fact that type arguments are erased
        throw throwAny(myUncheckedExceptions);
    }

    public static <T extends Throwable> T throwAny(Collection<T> throwables) throws T {
        // here the compiler will insert an implicit cast to T (just like your explicit one)
        // however, since T is a type variable, it gets erased to the erasure of its bound
        // and that happens to be Throwable, so a useless cast...
        throw throwables.iterator().next();
    }
}
Ben Schulz
  • 6,101
  • 1
  • 19
  • 15
  • `Class.newInstance()` and `Thread.stop(Throwable)` ultimately throw the exception from a `native` implementation. So that's probably fine with the JLS. Even if they *behave* the same (JVM doesn't care), to the JLS they're not the same (javac cares) as can be seen in various chapters, such as [`§11.2`](http://docs.oracle.com/javase/specs/jls/se7/html/jls-11.html#jls-11.2) for instance – Lukas Eder Sep 25 '12 at 10:13
  • 1
    @LukasEder Exceptions are part of the type system. If you cast away (no pun intended) the type information, the compiler has nothing to work with. You convinced it that there is no checked exception there. I might add that you did so with an unchecked cast which raises a warning, so I'm not sure what more you are asking for. – Ben Schulz Sep 25 '12 at 10:42
  • *"cast away"* ;-). Yes, that's what I was looking for. Is it really so, that I can legally convince the compiler that there are no checked exceptions involved? I think I can, for the reasons mentioned. But I'm surprised by this, so I wanted to ask the question. – Lukas Eder Sep 25 '12 at 11:33
  • 1
    @LukasEder Yes, it's the cast. A more elaborate way to do the same was to stuff it in a `List` and cast that to `List`. Then you can pull it back out an throw it as an unchecked exception. It's erasure. Again. :D – Ben Schulz Sep 25 '12 at 11:52
  • Awesome idea with the lists! Unfortunately, that seems to result in a `ClassCastException` when pulling out the `RuntimeException`... – Lukas Eder Sep 25 '12 at 11:53
  • @LukasEder Not if you do it in a context where the lists elements are `E`s. The upper bound on `E` will need to be a subtype of `Throwable` to allow the throw and a supertype of the contained exception to avoid the CCE. Anyways, the point was: Erasure is the source of the problem. – Ben Schulz Sep 25 '12 at 12:45
  • Can you show an example? That would make your answer the accepted answer – Lukas Eder Sep 25 '12 at 14:22
  • This suggestion cannot work, and if you get something resembling this to work, you'll see that the list involved is just a side-note and the main point will be exactly the same as in Lukas' example. – Marko Topolnik Sep 25 '12 at 14:54
  • @MarkoTopolnik It does "work" (the checked exception is thrown). And of course it's the same principle as in Lukas' example. The point is that the compiler is helpless because the unchecked cast that causes this mess can be burried anywhere. And *that* is the reason the JLS can't mandate anything stricter (answering the original question). – Ben Schulz Sep 25 '12 at 15:51
  • Yes, that's it: a method with the unchecked cast must compile into definite bytecode that is called regardless of the type params. That's why it is obvious that no compiler bugs are involved. – Marko Topolnik Sep 25 '12 at 17:15
2

This example is documented in The CERT Oracle Secure Coding Standard for Java which documents several non compliant code examples.

Noncompliant Code Example (Generic Exception)

An unchecked cast of a generic type with parameterized exception declaration can also result in unexpected checked exceptions. All such casts are diagnosed by the compiler unless the warnings are suppressed.

interface Thr<EXC extends Exception> {
  void fn() throws EXC;
}

public class UndeclaredGen {
static void undeclaredThrow() throws RuntimeException {
@SuppressWarnings("unchecked")  // Suppresses warnings
Thr<RuntimeException> thr = (Thr<RuntimeException>)(Thr)
  new Thr<IOException>() {
    public void fn() throws IOException {
      throw new IOException();
}
  };
thr.fn();
}

public static void main(String[] args) {
  undeclaredThrow();
 }
}

This works because RuntimeException is child class of Exception and you can not convert any class extending from Exception to RuntimeException but if you cast like below it will work

 Exception e = new IOException();
 throw (RuntimeException) (e);

The case you are doing is same as this. Because this is explicit type casting this call will result in ClassCastException however compiler is allowing it.

Because of Erasure however in your case there is no cast involved and hence in your scenario it does not throw ClassCastException

In this case there is no way for compiler to restrict the type conversion you are doing.

However if you change method signature to below it will start complaining about it.

static <E extends Exception> void doThrow0(E e) throws E {
Amit Deshpande
  • 19,001
  • 4
  • 46
  • 72
  • Your intuition is the same as mine. Note, the purpose of `doThrow0()` was to hide the unsafe cast from `Exception` to `E` in `doThrow()`... So changing the signature won't make sense here, for the sake of my example. – Lukas Eder Sep 25 '12 at 11:47
  • @LukasEder It works without Generics also. please check the example. – Amit Deshpande Sep 25 '12 at 11:51
  • But AmitD, your code **will** throw a `ClassCastException`, masking the exception you wanted to throw! – Marko Topolnik Sep 25 '12 at 11:51
  • @MarkoTopolnik `ClassCastException` is runtime behaviour but compile time behaviour is same. There is difference in both because of Erasure. – Amit Deshpande Sep 25 '12 at 11:55
  • But then you're missing the whole point---you didn't manage to write code that will actually **throw a checked exception it didn't declare**. – Marko Topolnik Sep 25 '12 at 11:57
  • @AmitD: Yes, try my example. There's no `ClassCastException` from my code. In Java 6 and 7, my example actually throws the checked exception (as can be seen in bytecode) – Lukas Eder Sep 25 '12 at 11:58
  • @LukasEder People [How to throw undeclared checked exception](http://java.dzone.com/articles/throwing-undeclared-checked) have discovered this before I wonder why there is no bug against it. – Amit Deshpande Sep 25 '12 at 12:30
  • Nice, method #5 is just the same as mine. Good to know – Lukas Eder Sep 25 '12 at 12:49
  • 1
    @LukasEder check the link to book this might be answer to your question. – Amit Deshpande Sep 25 '12 at 13:49
  • Thanks for the research. That's an interesting book. I guess seeing this example being referenced often means that it's an accepted behaviour and thus, probably not a javac bug – Lukas Eder Sep 25 '12 at 14:32
1

JLS 11.2:

For each checked exception which is a possible result, the throws clause for the method (§8.4.6) or constructor (§8.8.5) must mention the class of that exception or one of the superclasses of the class of that exception (§11.2.3).

This clearly indicates doThrow must have Exception in it's throws clause. Or, as downcasting (Exception to RuntimeException) is involved, the check must be done that Exception IS RuntimeException, which should fail in example because the exception being cast is SQLException. So, ClassCastException should be thrown in runtime.

As of practical side, this java bug allows to create a hack unsafe for any standard exception handling code, like next:

try {
 doCall();
} catch (RuntimeException e) {
 handle();
}

The exception will go up without being handled.

  • To me, *"possible result"* leaves room for interpretation. I'm arguing that if `` is bound to an unchecked exception, there is no such *"possible result"*. Do note that the unsafe cast `(E) e` is hidden within `doThrow0()`, and thus invisible to `doThrow()`. I'm not saying this is all good, but I don't see why the JLS is violated by these simple rules that don't even mention generics... – Lukas Eder Sep 25 '12 at 11:32
1

I don't think it can be disputed that the JLS allows that code. The question is not whether that code should be legal. The compiler simply does not have enough information to issue an error.

The issue here with the code emitted when you call a generic-throwing method. The compiler currently inserts type casts after calling a generic-returning methods where the returned value will be immediately assigned to a reference.

If the compiler wanted, it could prevent the problem by silently surrounding all generic-throwing method invocations in a try-catch-rethrow fragment. Since the exception would be caught and assigned to a local variable, a type cast would be mandatory. So you would get a ClassCastException if it was not an instance of RuntimeException.

But the spec does not mandate anything special about generic-throwing methods. I believe that's an spec bug. Why not file a bug about that, so that it can be fixed or at least documented.

By the way, the ClassCastException would suppress the original exception, which can result in hard-to-find bugs. But that problem has a simple solution since JDK 7.

Saintali
  • 4,482
  • 2
  • 29
  • 49
  • 1
    I agree javac should add a runtime type check to detect this problem early. tho it may not be always possible - if the target type is generic, check can't be done fully, the problem can be hidden till later. – irreputable Sep 25 '12 at 16:11
  • @irreputable right, if the target type is generic, it won't be cast immediately. But the caller itself being generic-throwing, can only be called as a generic-throwing. The compiler will have check this latter call site and wrap it in a try-catch-rethrow if necessary, so forth. – Saintali Sep 25 '12 at 16:17