What on earth is "Self-suppression not permitted" and why is Javac generating code which results in this error?

Question

This new Java 7 try-with-resources construct is quite nice. Or at least, it was nice until an exception came along and ruined my day.

I've finally managed to boil it down to a reproducible test which uses nothing but JUnit+jMock.

@Test
public void testAddSuppressedIssue() throws Exception {
    Mockery mockery = new Mockery();
    final Dependency dependency = mockery.mock(Dependency.class);

    mockery.checking(new Expectations() {{
        allowing(dependency).expectedCall();
        allowing(dependency).close();
    }});

    try (DependencyUser user = new DependencyUser(dependency)) {
        user.doStuff();
    }
}

// A class we're testing.
private static class DependencyUser implements Closeable {
    private final Dependency dependency;

    private DependencyUser(Dependency dependency) {
        this.dependency = dependency;
    }

    public void doStuff() {
        dependency.unexpectedCall(); // bug
    }

    @Override
    public void close() throws IOException {
        dependency.close();
    }
}

// Interface for its dependent component.
private static interface Dependency extends Closeable {
    void expectedCall();
    void unexpectedCall();
}

Running this example, I get:

java.lang.IllegalArgumentException: Self-suppression not permitted
    at java.lang.Throwable.addSuppressed(Throwable.java:1042)
    at com.acme.Java7FeaturesTest.testTryWithResources(Java7FeaturesTest.java:35)

Reading the documentation, they seem to be saying that if you were to add a suppressed exception back to itself, that is what triggers this error. But I'm not doing that, I'm just using a try-with-resources block. The Java compiler then generates what would seem to be illegal code, which makes the feature effectively unusable.

Of course, when the test passes, no problem occurs. And when the test fails, an exception occurs. So now that I have fixed the problem I originally discovered I have reverted to using try-with-resources. But next time an exception occurs, I would much rather the exception be the expectation failure, instead of one Java itself has emitted for seemingly no good reason.

So... is there a way to get proper error reporting here, without giving up on try-with-resources?

My first reaction would be to blame jMock rather than the Java compiler. — Stephen C, Aug 24 '12 at 05:48
For what it's worth, I think there's a fair chance this will be improved soon: [I started a discussion thread](http://old.nabble.com/Throwable.addSuppressed-error-conditions----use-the-exception-as-the-cause--td35270347.html) which [lead to a proposed patch](http://openjdk.5641.n7.nabble.com/Code-review-request-for-8012044-Give-more-information-about-self-suppression-from-Throwable-addSupprd-td127395.html). It only improves diagnostics, the underlying problem will still need to be fixed. The patch is not merged in yet, but it'll trickle down eventually. — Steven Schlansker, Apr 18 '13 at 03:54
Zhong Yu's reply on that thread is interesting and contradicts someone's claim in the comments of an answer below that reusing an exception object is supposed to be OK... who is right? Or is there a special case where reusing an exception object is OK unless it's thrown from close()? — Hakanai, Apr 19 '13 at 00:25
I have experienced this coming up in a framework we were writing for handling messages asynchronously. The framework makes use of CompletableFutures that run on dedicated thread pools and try-with-resources for the framework. We then saw this exception coming up. The issue was that we weren't handling exceptions correctly in our framework but the issue is aggravated by the JVM. So both arguments are correct from different angles. Thanks for highlighting the JVM issue @StevenSchlansker — Luke Machowski, Oct 12 '17 at 10:04

score 7 · Accepted Answer · answered Aug 24 '12 at 05:51

7

It looks like jMock throws the same instance of exception from the both methods. That's how it can be reproduced without jMock:

public class Test implements Closeable {
    private RuntimeException ex = new RuntimeException();

    public void doStuff() {
        throw ex;
    }

    public void close() {
        throw ex;
    }
}

try (Test t = new Test()) {
    t.doStuff();
}

If so, I think it's a problem of jMock rather than of Java compiler.

answered Aug 24 '12 at 05:51

axtavt

239,438
41
511
482

Yeah, this is exactly it. The jMock expectation fails when `unexpectedCall()` is invoked on the mocked object, and `Mockery.dispatch()` rethrows the same exception instance on subsequent calls like the one that gets made for `close()`. – Tim Stone Aug 24 '12 at 06:25
I was able to reproduce it this way as well. But I couldn't find the bit in the JLS where it says that it was illegal to reuse Exception instances. I assume that this is the case? – Hakanai Aug 24 '12 at 06:40
http://stackoverflow.com/questions/11473263/ suggests that sharing exception instances is OK. – Hakanai Aug 24 '12 at 06:43
2

@Trejkaz There's nothing wrong with reusing the instance in general, you just can't pass a self-reference to [`addSuppressed()`](http://docs.oracle.com/javase/7/docs/api/java/lang/Throwable.html#addSuppressed(java.lang.Throwable)). Because the exception in `close()` didn't *cause* the exception within the try-with-resources block, the `close()` exception instead gets added to the suppression list of the exception from `unexpectedCall()`. Due to the reuse here, this causes the exception to attempt to suppress itself, which isn't allowed per the method description. – Tim Stone Aug 24 '12 at 12:31
Right. So this is a Java compiler issue, in that it isn't checking whether it's the same instance before adding it as the suppressed exception. :( – Hakanai Aug 27 '12 at 00:48
@Trejkaz To clarify for anyone coming here: It is *not* a compiler issue. The compiler can't do anything about this, because the requirement for non-self-surpression is not visible anywhere in the method signature, only in the code itself and in the documentation. Even if it were, it cannot then analyze the whole logic of which exception could be thrown where and whether it is the same instance or not. You *cannot* require a compiler to reason about subtleties like this. Again, the compiler shouldn't check, the library should, and it *does*. – Adowrath Sep 15 '17 at 09:28
1

@Adowrath all the compiler would have to do is generate additional code to check whether it's the same instance before adding it. So the compiler absolutely _can_ do something about it - it just doesn't. – Hakanai Sep 16 '17 at 11:10
@Trejkaz Yes, it could. It *could* do many things. But a) that doesn't mean it *should* do those things (it is an **extremely** rare edge case which does not warrant individual coverage), nor b) was that what you meant by "in that it isn't checking whether it's the same instance", which suggested to me that you'd want a constant, static analysis of this case, and not an additional runtime check. Amd what would you suggest it do in the case of `close()` throwing the same exception again? Generate a new `RepeatedExceptionInFinallyClause` exception and throw that instead, containing the original? – Adowrath Sep 16 '17 at 11:30
1

That would work, but you could almost just ignore the second occurrence since the exact same exception has already been reported. – Hakanai Sep 17 '17 at 11:59
@Trejkaz And add that check to every finally clause's suppression? What *exactly* is the added benefit? Yes, you do not get a different error than you wanted/suspected when writing this exception reuise.. But there is a bytecode and a small runtime increase, complication in the implementation (this would have to be documented in the JLS). IMHO, this should not be a compiler change. It could however be a reasonable change in the addSuppressed method, where attempted self-suppression would just *add* an indication of that to the stack trace. Features should carry their own weight. This wouldn't. – Adowrath Sep 17 '17 at 14:37

eckes · Answer 2 · 2014-05-03T01:14:59.463

I had a problem in Apache Commons VFS (Unit Test failed on Java 8, see VFS-521). And it turns out that java.io.FilterOutputStream is using the try-with-resource (suppressed exception) feature in a way that it cannot deal with flush+close throwing the same exception.

And what is even worse, before Java 8 it just silently swallows exceptions from the flush() call, see JDK-6335274).

I fixed it, by avoiding super.close() at all. Currently discussing this on the corelibs-dev openjdk mailingl ist: http://openjdk.5641.n7.nabble.com/FilterOutputStream-close-throws-exception-from-flush-td187617.html

What on earth is "Self-suppression not permitted" and why is Javac generating code which results in this error?

2 Answers2

Linked