12 Replies Latest reply on Dec 8, 2010 7:45 AM by adinn

    Exception thrown by rule is not caught by woven code

    flavia.rainone

      I wrote a test for JBoss MSC that forces ServiceInstanceImpl to enter the catch RejectedExecutionException block:

       

       

      void doExecute(final Runnable... tasks) {
              assert ! lockHeld();
              if (tasks == null) return;
              final Executor executor = primaryRegistration.getContainer().getExecutor();
              for (Runnable task : tasks) {
                  try {
                      executor.execute(task);
                  } catch (RejectedExecutionException e) {
                      task.run();
                  }
              }
          }
      

       

      Here is my rule:

       

       

      RULE Throw RejectedExecutionException
      CLASS org.jboss.msc.service.ServiceInstanceImpl
      METHOD doExecute
      AT INVOKE java.util.concurrent.Executor.execute
      BIND NOTHING
      IF TRUE
      DO
         debug("Rejecting execution"),
         throw new java.util.concurrent.RejectedExecutionException();
      ENDRULE
      

       

       

      The problem is that the code does not enter the catch block and the tests fail because of the uncaught exception. Here is the trace:

       

      java.util.concurrent.RejectedExecutionException
          at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
          at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:39)
          at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:27)
          at java.lang.reflect.Constructor.newInstance(Constructor.java:513)
          at org.jboss.byteman.rule.expression.ThrowExpression.interpret(ThrowExpression.java:242)
          at org.jboss.byteman.rule.Action.interpret(Action.java:144)
          at org.jboss.byteman.rule.helper.InterpretedHelper.fire(InterpretedHelper.java:169)
          at org.jboss.byteman.rule.helper.InterpretedHelper.execute0(InterpretedHelper.java:137)
          at org.jboss.byteman.rule.helper.InterpretedHelper.execute(InterpretedHelper.java:100)
          at org.jboss.byteman.rule.Rule.execute(Rule.java:664)
          at org.jboss.byteman.rule.Rule.execute(Rule.java:633)
          at org.jboss.msc.service.ServiceInstanceImpl.doExecute(ServiceInstanceImpl.java:392)
          at org.jboss.msc.service.ServiceInstanceImpl.internalSetMode(ServiceInstanceImpl.java:549)
          at org.jboss.msc.service.ServiceInstanceImpl.setMode(ServiceInstanceImpl.java:412)
          at org.jboss.msc.service.ServiceContainerImpl.commit(ServiceContainerImpl.java:433)
          at org.jboss.msc.service.ServiceContainerImpl.install(ServiceContainerImpl.java:359)
          at org.jboss.msc.service.ServiceContainerImpl.install(ServiceContainerImpl.java:510)
          at org.jboss.msc.service.ServiceBuilderImpl.install(ServiceBuilderImpl.java:287)
          at org.jboss.msc.service.DependencyListenersTestCase.testMissingDependencies(DependencyListenersTestCase.java:59)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at java.lang.reflect.Method.invoke(Method.java:597)
          at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
          at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
          at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
          at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
          at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
          at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:31)
          at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
          at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
          at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
          at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
          at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
          at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
          at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
          at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
          at org.junit.runners.Suite.runChild(Suite.java:128)
          at org.junit.runners.Suite.runChild(Suite.java:24)
          at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
          at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
          at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
          at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
          at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
          at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
          at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:49)
          at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
          at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)
      

       

      Weird enough, I debugged it and it seems it should work... is there any explanation for this behavior? And, most important, is there a way to make my test work correctly?

        • 1. Re: Exception thrown by rule is not caught by woven code
          adinn

          Hi Flavia,

           

          You can do what you want but not with this rule. The problem is that you are misreading the semantics of the THROW action. When a THROW is executed by an injected rule the throw does not happen at the trigger point. The exception is thrown from the trigger method call. Since your rule is attached to method doExecute the rule causes an immediate exceptional exit from doExecute. The user guide describes this as 'short-circuiting' the rest of the method call and that includes bypassing any internal catch or finally blocks. The same applies if you had placed a RETURN in the action -- it means don't execute any more trigger method code, just return immediately from the trigger method call.

           

          I chose this semantics for a good reason. In case you (or anyone else reading this) are interested here is why. If not skip ahead for the workaround

           

          [begin technical  diversion]

          The problem with throwing the exception from the trigger injection point is that it may not be type safe to do so. There is no easy way of knowing whether this will produce valid bytecode. The throw may be in a catch block and this may be embedded in a wider try catch cascade or even a try catch hierarchy. So, just throwing an exception may cause all sorts of unexpected outcomes. In particular an infinite loop might arise as many handler blocks are re-entered if the same exception is rethrown. Byteman cannot simply rearrange the exception flow during injection to cater for a new exception at the trigger point. That requires the ability to do type analysis during injection and this is very hard to do using the transformer API (it normally requires class-loading which breaks transformation).

           

          So, I decided to implement throw and return as a short-circuit. The trigger method has a definite return and exception type signature so returning a value or throwing an exception from the trigger method allows type-checking against the method contract. Also, it is possible, although tricky, to ensure that Byteman generated RETURNs or THROWs are routed out of the method's handler hierarchy to a top-level throw or exit without the need for complex type analysis. The rule wraps the return value or thrown object in an exception private to Byteman. The injected code region is guarded with a top-level  handler for this exception which unwraps the internal exception and either returns  or throws from  the  top  level. If you want  to see how this works you can dump the injected code and decompile it to have a look (set -Dorg.jboss.byteman.dump.generated.classes on the JVM command line).

          [end technical diversion]

           

          There is a way to achieve the behaviour you want. You need to throw an exception from Executor.execute when it is called underneath ServiceInstanceImpl.doExecute

           

          RULE throw exception from execute under doExecute
          CLASS Executor
          METHOD execute
          IF callerEquals("ServiceInstanceImpl.execute", true)
          DO debug("rejecting execution");
              throw new java.util.concurrent.RejectedExecutionException()
          ENDRULE
          

           

          The built-in method callerEquals checks up the stack to see what the calling method is. It has various argument patterns. The first argument is always the name of a caller method to be searched for. The second argument says include the class name when doing the comparison (add another true argument  and it will inlcude the package too). With no further arguments the test just checks the immediate caller of the trigger method. So, this rule wiill fire when the immediate caller is ServiceInstanceImpl.doExecute and throw the exception  from the call  to Executor.execute which is what you want.

          • 2. Re: Exception thrown by rule is not caught by woven code
            flavia.rainone

            I used the proposed fix but it didn't work. I did some changes, too, but still no luck:

            RULE throw exception from execute under doExecute
            INTERFACE java.util.concurrent.Executor
            METHOD execute
            IF TRUE
            #callerEquals("org.jboss.msc.service.ServiceInstanceImpl.doExecute", true)
            DO debug("rejecting execution"),
                throw new java.util.concurrent.RejectedExecutionException();
            ENDRULE
            

             

            The verbose output does not show anything... I even wondered if I was making some mistake at the moment of executing Byteman, such as command line misconfiguration, but apparently not. If I add another rule, the other rule is recognized by Byteman. On the other hand, if I introduce some sintax error to the rule above, such as removing the E from IF TRUE, Byteman does not complain at all.

             

            To me, the explanation is that Byteman doesn't weave java.* classes... which is what I expected in the first place, and that's why I was using the AT INVOKE of the first post. Is this assumption correct?

            • 3. Re: Exception thrown by rule is not caught by woven code
              adinn

              Flavia Rainone wrote:

               

              To me, the explanation is that Byteman doesn't weave java.* classes... which is what I expected in the first place, and that's why I was using the AT INVOKE of the first post. Is this assumption correct?

              Ah, sorry, I assumed you were aware of how  to deal with this issue. Byteman will inject into java.* classes but only if you ask it to. In fact, you need to ask nicely because Byteman has to do two things for this to work.

               

              First you need to pass

               

              -Dorg.jboss.byteman.transform.all

               

              on the JVM command line. Byteman will  only inject  into java.*  code if you set this property.

               

              Second, you need to ensure  that the injected code is able to resolve the Byteman exception types located in the byteman jar. Agent jars are normally added to the system classpath. But java.* classes may be in the botostrap classpath. So, to fix this you need to ensure that Byteman code gets loaded from the botstrap classpath when the agent is started.You can do this using the boot: option to the -javaagent flag

               

              -javaagent:/path/to/byteman.jar=boot:/path/to/byteman.jar,script:/other/path/to/myscript.txt

               

              on the JVM  command line. Altermatively, if you are using bminstall.sh to install the agent into a running program you pass flag -b.

              • 4. Re: Exception thrown by rule is not caught by woven code
                flavia.rainone

                Great! That solved the problem, with one minor detail: I had to replace the "INTERFACE java.util.concurrent.Executor" by " CLASS java.util.concurrent.ThreadPoolExecutor".

                This is bad to me, because the test shouldn't be aware that the executor currently used by ServiceInstanceImpl is a ThreatPoolExecutor. If that changes in the future, the test will be broken, and to top it off, the test assertions all pass when the rule is ignored.

                I'll add a safeguard that makes the test fail in such cases, but I wonder why the rule didn't work with INTERFACE... am I missing something again?

                • 5. Re: Exception thrown by rule is not caught by woven code
                  adinn

                  Flavia Rainone wrote:

                   

                  Great! That solved the problem, with one minor detail: I had to replace the "INTERFACE java.util.concurrent.Executor" by " CLASS java.util.concurrent.ThreadPoolExecutor".

                  This is bad to me, because the test shouldn't be aware that the executor currently used by ServiceInstanceImpl is a ThreatPoolExecutor. If that changes in the future, the test will be broken, and to top it off, the test assertions all pass when the rule is ignored.

                  I'll add a safeguard that makes the test fail in such cases, but I wonder why the rule didn't work with INTERFACE... am I missing something again?

                   

                  Afraid so. If you use

                   

                  INTERFACE java.util.concurrent.Executor
                  METHOD execute
                  

                   

                  then your rule will be injected into  every class  which declares that it implements java.util.concurrent.Executor.execute(). The declaration for ThreadpoolExecutor is as follows

                   

                  public class ThreadPoolExecutor extends AbstractExecutorService
                  {
                    ...

                   

                  which does not declare that it implements Executor. Instead it overrides class AbstractExecutorService which does declare that it implements Executor. So, you rule gets considered  for injection into class AbstractExecutorService but not for class ThreadPoolExecutor.

                   

                  If you want injection to be performed down the class hierarchy into overriding methods then Byteman will do it but you have to ask for it. All you need to do is prefix the name in the CLASS/INTERFACE clause with a ^ i.e. in your case the rule should be

                  RULE throw exception from execute under doExecute
                  INTERFACE ^java.util.concurrent.Executor
                  METHOD execute
                  IF callerEquals("org.jboss.msc.service.ServiceInstanceImpl.doExecute", true, true)
                  DO debug("rejecting execution"),
                      throw new java.util.concurrent.RejectedExecutionException();
                  ENDRULE
                  • 6. Re: Exception thrown by rule is not caught by woven code
                    flavia.rainone

                    Andrew, apparently reading once the manual was not enough... I guess I'll reread it some time, too many details to get at once . I obviously missed several important features here.

                     

                    So, I followed your suggestion but, unfortunately, the ^ is not working in my case... I debugged it, and it seems that Transformer.transform(ClassLoader, String, Class<?>, ProtectionDomain, byte[]) should search for the interfaces extedended by interfaces, at the for block that starts at line 290. The link from ExecutorService interface to its super interface Executor is being missed.

                    • 7. Re: Exception thrown by rule is not caught by woven code
                      adinn

                      Flavia Rainone wrote:

                       

                      Andrew, apparently reading once the manual was not enough... I guess I'll reread it some time, too many details to get at once . I obviously missed several important features here.

                       

                      Hmm yes, I think the feature set has now probably grown to the point that a revision of the manual might be helpful. It was described as refreshingly clear about a year ago but these things change as stuff gets bolted on.

                       

                      So, I followed your suggestion but, unfortunately, the ^ is not working in my case... I debugged it, and it seems that Transformer.transform(ClassLoader, String, Class<?>, ProtectionDomain, byte[]) should search for the interfaces extedended by interfaces, at the for block that starts at line 290. The link from ExecutorService interface to its super interface Executor is being missed.

                      Oh yes, of course. I should have thought of that. Thank you very much for spotting it. I have raised a JIRA for this issue.

                       

                          https://jira.jboss.org/browse/BYTEMAN-140

                       

                      Your case reveals  a very interesting failing which raises some interesting questions about the rule semantics. This is maybe best explained with an example. Imagine we have the following situation

                       

                      public interface I'
                      {
                        public void foo()
                        ...
                      
                      public interface I extends I'
                      {
                        ...
                      public class C implements I
                      {
                        public void foo()
                        ...
                      public class S extends C
                      {
                        public void foo()
                        ...
                      

                       

                      Ok,  so your bug arises when we have a rule like this:

                       

                      RULE overriding interface rule
                      INTERFACE ^I'
                      METHOD foo
                      ...

                       

                      This rule clearly needs to be considered for injection into S and C since they both implement a method of I' and the rule requests injection into overriding methods. Your example here has made it clear that neither S nor C will be considered. Fixing the block at line 290 so it checks the full interface hierarchy fromthe super class upwards will ensure that S is considered but will not do so for C. The  interfaces implemented by C only get checked in the previous block at line 252. This could be fixed by making the block at line 290 also check the super interfaces of the interfaces implemented by C. But is this actually what is needed?

                       

                      What about when we have a rule like this?

                       

                       

                      RULE generic interface rule
                      INTERFACE I'
                      METHOD foo

                      ...

                       

                      Clearly, this rule should not be injected into S because S does not directly implement I or I'. Should it be injected into C? Strictly, C does not implement I', it only implements I. One of the ways I have described the semantics of INTERFACE rules without a ^ is to say that they are only injected into classes which directly implement the interface. So, this would mean that the generic interface  rule should not be injected into C. However, I also described the ^ as  requesting injection into overriding methods rather than methods which directly implement the interface. In the S < C < I < I' hierarchy C is the first place where a method of I or I' can be implemented. The implementations on C don't override another implementation. By contrast, S can only implement methods of I or I' by overriding implememntations provided by C. So, looking at it this way it seems appropriate to inject the generic interface  rule into class C.

                       

                      It is interesting to  consider why this arises. The keywords extends and implements actually hide  some important details here. There are actually three relations at stake here

                       

                      extends : Class --> Class

                      extends : Interface --> Set(Interface)

                      implements : Class --> Set(Interface)

                       

                      The first relationship defines a forest of trees representing the subclass hierarchy. The second defines a forest of directed acyclic graphs (DAGs) defining the interface hierarchy. The third defines a boundary transition set of links connnecting the forest of subclass trees into the forest of interface DAGs. So, the choice I presented above is about which subset of these links an INTERFACE rule applies to when considering a given class. The first choice says inject into C if the interface in the rule can be reached imediately from C via a direct link in the boundary transition set. The second choice says inject into C if the interface in the rule can be reached immediately or indirectly via a direct link in the boundary transition set. An INTERFACE ^ rule says inject into C (or S) if the interface in the rule is reachable by any upward path from C (or S).

                       

                      Personally, I prefer the second choice for INTERFACE rules.  However, I am not yet totally committed to this choice. Do you have any opinions on this?

                      • 8. Re: Exception thrown by rule is not caught by woven code
                        flavia.rainone

                        Andrew Dinn wrote:

                         

                        It is interesting to  consider why this arises. The keywords extends and implements actually hide  some important details here. There are actually three relations at stake here

                         

                        extends : Class --> Class

                        extends : Interface --> Set(Interface)

                        implements : Class --> Set(Interface)

                         

                        The first relationship defines a forest of trees representing the subclass hierarchy. The second defines a forest of directed acyclic graphs (DAGs) defining the interface hierarchy. The third defines a boundary transition set of links connnecting the forest of subclass trees into the forest of interface DAGs. So, the choice I presented above is about which subset of these links an INTERFACE rule applies to when considering a given class. The first choice says inject into C if the interface in the rule can be reached imediately from C via a direct link in the boundary transition set. The second choice says inject into C if the interface in the rule can be reached immediately or indirectly via a direct link in the boundary transition set. An INTERFACE ^ rule says inject into C (or S) if the interface in the rule is reachable by any upward path from C (or S).

                         

                        Personally, I prefer the second choice for INTERFACE rules.  However, I am not yet totally committed to this choice. Do you have any opinions on this?

                        I also prefer the second choice. Otherwise, if somebody wants to apply a rule strictly to classes like C, this user will have no choice unless state the name of the classes.

                         

                        I also think this change could hardly impact current users... IMHO, if somebody ran into this before, chances are the user would have raised the issue on the forum, thinking it could be a bug.

                        • 9. Re: Exception thrown by rule is not caught by woven code
                          adinn

                          Flavia Rainone wrote:

                           

                          I also prefer the second choice. Otherwise, if somebody wants to apply a rule strictly to classes like C, this user will have no choice unless state the name of the classes.

                           

                          I also think this change could hardly impact current users... IMHO, if somebody ran into this before, chances are the user would have raised the issue on the forum, thinking it could be a bug.

                          Sure, this is probably the first occasion anyone has noticed this. It's a fairly esoteric case.

                           

                          Interestingly, your rationale for choosing option 2 was also mine. I'll take that as reliable confirmation that this is the right track and fix the JIRA and docs accordingly. Explaining the new semantics is going to be an interesting challenge to the readability of the manual :-).

                           

                          Thanks very much for your input and, especially, for locating this bug. It's so nice to have power-users.

                           

                          regards,

                           

                           

                          Andrew Dinn

                          • 10. Re: Exception thrown by rule is not caught by woven code
                            adinn

                            Hi Flavia,

                             

                            I have fixed the bug and added a bugfix test to the test suite to check that it works. It seems to do  the job correctly now.

                             

                            If you weant  to try it out on your test case you can download trunk from svn and build it by executing 'ant install'. Otherwise just wait for the next patch/update release.

                             

                            regards,

                             

                             

                            Andrew Dinn

                            • 11. Re: Exception thrown by rule is not caught by woven code
                              flavia.rainone

                              Great, Andrew! I just checked it out and now the test pass with:

                               

                              INTERFACE ^java.util.concurrent.Executor
                              

                               

                              I'm looking forward to the next release :-)

                              • 12. Re: Exception thrown by rule is not caught by woven code
                                adinn

                                Flavia Rainone wrote:

                                 

                                Great, Andrew! I just checked it out and now the test pass with:

                                 

                                INTERFACE ^java.util.concurrent.Executor
                                

                                 

                                I'm looking forward to the next release :-)

                                 

                                Excellent! and thank you for confirming  that the fix works ok.

                                 

                                The next planned release is a feature release, 1.5.0. I was not going to schedule it until I came up with some new things to add but I have no new tricks up my sleeve just now :-(. I may create a 1.4.2 patch release in the next month, especially if I can find and fix a few more bugs by then. So, keep breaking the product ;-)