1 2 Previous Next 15 Replies Latest reply on Jul 29, 2006 9:33 AM by Tim Fox

    Justification for "throws Throwable"

    Tim Fox Master

      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?

        • 1. Re: Justification for
          Scott Stark Master

           

          "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.

          • 2. Re: Justification for
            Tim Fox Master

            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.

            • 3. Re: Justification for
              Scott Stark Master

              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.


              • 4. Re: Justification for
                Tim Fox Master

                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.

                • 5. Re: Justification for
                  Adrian Brock Master

                  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! :-)

                  • 6. Re: Justification for
                    Tim Fox Master

                    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?




                    • 7. Re: Justification for
                      Tim Fox Master

                       

                      "adrian@jboss.org" wrote:
                      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)?

                      • 8. Re: Justification for
                        Adrian Brock Master

                         

                        "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

                        • 9. Re: Justification for
                          Tim Fox Master

                           

                          "adrian@jboss.org" wrote:

                          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?

                          • 10. Re: Justification for
                            Tim Fox Master

                            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.

                            • 11. Re: Justification for
                              Scott Stark Master

                               

                              "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();
                               }
                              
                              }
                              



                              • 12. Re: Justification for
                                Adrian Brock Master

                                 

                                "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()?

                                • 13. Re: Justification for
                                  Scott Stark Master

                                   

                                  "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.

                                  • 14. Re: Justification for
                                    Tomohisa Igarashi Newbie

                                     

                                    "scott.stark@jboss.org" wrote:
                                    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.

                                    1 2 Previous Next