Justification for "throws Throwable"

[timfox]

A few of the older methods in the core are declared as "throws Throwable".

I have been told there was some reason for doing this but that reason seems to have been lost in the mists of time.

Declaring as throws Throwable forces the caller to handle RuntimeExceptions and Errors.

Can anyone shed any light on this?

[starksm64]

"timfox" wrote:

Declaring as throws Throwable forces the caller to handle RuntimeExceptions and Errors.

Exactly this. In some places not handling unchecked exceptions can lead to thread leaks, connection leaks, etc. If an excecution path requires deterministic behavior there is no such thing as an unchecked exception in that path.

[timfox]

Can you describe a concrete example where you might want to declare a method as such?

I'm trying to understand how this would work in practice.

[starksm64]

Its tied to resource cleanup, be it a thread, socket, file descriptor, memory, ...:

Resource r = null;
try
{
 r = ...;
}
catch(Throwable t)
{
 if( r != null )
 r.close();
}

A method that declares a Throwable is an indication of a potential resource leak, or state transition inconsistency. More generally the issue is not having a complete state machine model of the subsystem and not explicitly dealing with the subsystem state when an error occurs. In this model unchecked vs checked exceptions related to whether or not the current system state can safely ignore the exception and transition correctly to the next state. If it can the exception can be unchecked. If it cannot the exception is checked and needs to be caught.

[timfox]

That make sense, although I don't see how that applies to the particular methods that have been declared as throws Throwable in messaging core.

[adrian.brock]

Resource allocation can generally be dealt with try/finally.
The "throws Throwable" is more for when you probably need
to perform some reversing operation in your state, e.g.

step1();
try
{
 step2();
}
catch (Throwable t)
{
 reverseStep1();
 handle(t);
}

Of course step2() doesn't need to explicitly declare "throws Throwable"
but it does help to make you think about it! :-)

[timfox]

Wouldn't it be better if the method is written to always guarantee to leave the object in a well defined state?

If the object is not left in a well defined state then even if you catch Throwable, then there's not much you can do since you don't know if the underlying resource (or whatever) was actually allocated.

E.g.

Connection getConnection() throws Throwable
{

 log.info("before allocation");

 Connection conn = ......

 log.info("after allocation");

 return conn;
}

In the above, both calls to log.info can throw a RuntimeException. If it's thrown from the first log.info line then the connection hasn't been allocated, but if it's thrown from the second one then the connection has been allocated.

If you do something like:

Connection getConnection()
{
 Connection conn = null;

 try
 {

 log.info("before allocation");

 conn = ......

 log.info("after allocation");
 }
 catch (Throwable t)
 {
 if (conn != null) conn.close();

 throw new IllegalStateException();
 }

 return conn;
}

Then your're not leaving anything to chance and you can avoid throws Throwable.

This is mentioned in Bloch's effective Java "Strive for failure atomicity"

What am I missing?

[timfox]

Resource allocation can generally be dealt with try/finally.
The "throws Throwable" is more for when you probably need
to perform some reversing operation in your state

Ok, but if you get step1() to automatically reverse itself when a Throwable is thrown, then you don't need to worry about it. Isn't that easier (and safer)?

[adrian.brock]

"timfox" wrote:
That make sense, although I don't see how that applies to the particular methods that have been declared as throws Throwable in messaging core.

Because if everything delcares Throwable,
you can propogate the Throwable throughout your
internal api without wrapping it at every level.

e.g. for JMS you would wrap the Throwable in a JMSException
in the facade.

You probably aren't aware of the problems we had
due to the stupid jmx exception wrapping. :-)

This is mentioned in Bloch's effective Java "Strive for failure atomicity"

Not everything can be atomic.
e.g. in the example above, it could be JMS (outside a transaction)
step1 == write the message to a persistent store
step2 == do the processing to add to the queue
a failure in step2 requires
reverseStep1 == remove from the persistent store

[timfox]

Because if everything delcares Throwable,
you can propogate the Throwable throughout your
internal api without wrapping it at every level.

e.g. for JMS you would wrap the Throwable in a JMSException
in the facade.

Why do RuntimeExceptions/Errors need to be wrapped in a JMSException and sent to the client?

Can't we just propagate the RuntimeExceptions/Error as is to the client?

[timfox]

The philosophy I'm following is that JMSExceptions are for recoverable conditions, e.g. trying to send to a destination that doesn't exist, the database isn't up etc.

I'm treating RuntimeExceptions as programing errors - symptoms of some bug in the code - in which case we just want to throw it as is to the client.

[starksm64]

"timfox" wrote:
Wouldn't it be better if the method is written to always guarantee to leave the object in a well defined state?

If the object is not left in a well defined state then even if you catch Throwable, then there's not much you can do since you don't know if the underlying resource (or whatever) was actually allocated.

...

Then your're not leaving anything to chance and you can avoid throws Throwable.

This is mentioned in Bloch's effective Java "Strive for failure atomicity"

What am I missing?

You have made an assumption that all Throwables map to an IllegalStateException and that the actual type of the exception event is not relevant to the caller's state machine with respect to how it relates to obtaining a connection. I would miss the opportunity to have different transitions for different types of exception events:

{
 Connection conn = null;

 try
 {

 log.info("before allocation");

 conn = ......

 log.info("after allocation");
 }
 catch(NoRouteToHostException e)
 {
...
 }
 catch(OutOfMemoryError e)
 {
...
 }
 catch (Throwable t)
 {
 if (conn != null) conn.close();
 // Why should I throw away the type and prevent my caller from doing fine-grained event handling?
 throw new IllegalStateException();
 }

}

[adrian.brock]

"timfox" wrote:

Why do RuntimeExceptions/Errors need to be wrapped in a JMSException and sent to the client?

Can't we just propagate the RuntimeExceptions/Error as is to the client?

Errors yes/maybe - they are normally non-recoverable,
but that doesn't mean you shouldn't try, otherwise
the whole system could go into an indeterminate state.
There are many errors that might only be related to a specific request,
e.g. NoClassDefFoundError when unmarshalling an object message.

However, if the user passes bad parameters
then the appropriate exception should be thrown.

Any RuntimeException after you accepted the request is your problem,
it should be reported to the user as a JMSException, even
though it is likely fatal.

How do you pass that Error/RuntimeException to onException()?

[starksm64]

"timfox" wrote:
The philosophy I'm following is that JMSExceptions are for recoverable conditions, e.g. trying to send to a destination that doesn't exist, the database isn't up etc.

I'm treating RuntimeExceptions as programing errors - symptoms of some bug in the code - in which case we just want to throw it as is to the client.

This gets back to another question on whether implementation details should leak to clients. Given that there is no agreement on checked/unchecked exception usage, there is nothing semantically meaningful about a RuntimeException in terms of whether or not its a recoverable condition. It's just as likely to be a normal failure mode as an Exception depending on the frameworks in use on the server.

[igarashit]

This gets back to another question on whether implementation details should leak to clients.

I am afraid that.
If The version of the framework leaks to malicious client from stacktrace, that may cause any security broblem.