Type not declared by trigger method

[sannegrinovero]

Hello, I'm attempting to use Byteman to have all implementors of the java.sql.DatabaseMetaData interface throw a new java.sql.SQLException everytime the getDatabaseMajorVersion() method is invoked.

context: some JDBC driver versions are broken and don't implement this method throwing a SQLException when invoked. Not only old weird drivers, but even some recent versions of Oracle and PostgreSQL drivers; Hibernate invokes this method only to log some information about the database it's connecting to, so not critically important, but currently fails to boot when this exception is thrown.

It could easily log a warning instead and just, but I need an automated test to make the fix future-re-factoring proof.

There are some subtle differences in which this code is handled according to the database being used; continuous integration runs all functional tests on all supported databases so I would like to have this fault injection to be enabled on all JDBC implementations. (example: http://opensource.atlassian.com/projects/hibernate/browse/HHH-5642 - there are many more reports on other databases)

So my rule is:

RULE DatabaseMetaData fails getDatabaseMajorVersion()

INTERFACE ^java.sql.DatabaseMetaData

METHOD getDatabaseMajorVersion

IF TRUE

      DO throw new java.sql.SQLException("Byteman injected fault");

ENDRULE

but this throws:

org.jboss.byteman.agent.Transformer : possible trigger for rule ConcurrentMergeErrorHandledTest in class org.h2.jdbc.JdbcDatabaseMetaData RuleTriggerMethodAdapter.injectTriggerPoint : inserting trigger into org.h2.jdbc.JdbcDatabaseMetaData.getDatabaseMajorVersion() int for rule ConcurrentMergeErrorHandledTest org.jboss.byteman.agent.Transformer : inserted trigger for ConcurrentMergeErrorHandledTest in class org.h2.jdbc.JdbcDatabaseMetaData Rule.execute called for ConcurrentMergeErrorHandledTest_0 Rule.ensureTypeCheckedCompiled : error type checking rule ConcurrentMergeErrorHandledTest org.jboss.byteman.rule.exception.TypeException: ThrowExpression.typeCheck : exception type not declared by trigger method java.sql.SQLException file /home/sanne/workspaces/hibernate/hibernate-core-parent/testsuite/src/test/resources/bytemanrules.txt line 42

  The problem being that the method signature in the interface declares that it could throw a SQLException, but the actual implementation of H2's JDBC driver does not declare the same exception.

So I guess it's not possible to break this typesafety, but could I take advantage of it?

I think I could assume that all drivers not declaring it can't possibly ever throw it, so I would like to skip the rule if it's not possible to apply it, without failing the test. Ideally I'd love it to invoke a custome helper method, that would be useful to report in some way which tests where skipped.

[adinn]

Hi Sanne,

. . .

So my rule is:

RULE DatabaseMetaData fails getDatabaseMajorVersion()

INTERFACE ^java.sql.DatabaseMetaData

METHOD getDatabaseMajorVersion

IF TRUE

      DO throw new java.sql.SQLException("Byteman injected fault");

ENDRULE

but this throws:

org.jboss.byteman.rule.exception.TypeException: ThrowExpression.typeCheck : exception type not declared by trigger method java.sql.SQLException file /home/sanne/workspaces/hibernate/hibernate-core-parent/testsuite/src/test/resources/bytemanrules.txt line 42

  The problem being that the method signature in the interface declares that it could throw a SQLException, but the actual implementation of H2's JDBC driver does not declare the same exception.

So I guess it's not possible to break this typesafety, but could I take advantage of it?

I think I could assume that all drivers not declaring it can't possibly ever throw it, so I would like to skip the rule if it's not possible to apply it, without failing the test. Ideally I'd love it to invoke a custome helper method, that would be useful to report in some way which tests where skipped.

Yes, I think we can treat the case where the interface declares the checked exception but the implementaton does not declare it as a mismatch between the rule specification and the target method implementation and not classify it as an error.

This is much the same as the situation where a rule injected into CLASS ^Foo METHOD bar() mentions a local variable called $myVar.  If we have two classes Foo1 extends Foo and Foo2 extends Foo then when Foo1.bar() includes a variable called myVar Byteman injects the rule but when Foo2.bar() does not include a varibale called myVar Byteman does not throw a type error. The specification for the injected code provided in the rule applies in one case but not in the other so Byteman just ignores the second case. I think these sort of mismatches should be flagged with a warning but they should not cause a type error.

I can easily make the type checker smart enough to check the interface and implementation and decide that the rule does not apply. The problem is not working out how to do this but doing it at the right time. Ideally we should identify this situation before injecting the rule and avoid injecting into an implementation whcih does nto declare the thrown exception type. This is not straightforward to do because type checking normally happens inside the rule execution engine the first time a rule is executed i.e. after injection has already happened.

One possibility would be to inject the rule into the implementation method as normal, type check when it is first triggered and if we find the thrown exception is not declared by the implementation method ensure that an empty stub is executed instead of the rule. This means we still have the overhead of triggering the rule every time it gets called (i.e. executing the injected code which calls out to the rule execution engine) but none of the cost of executing the BIND IF or DO clauses. It's not perfect but it will give the desired behaviour

I will prototype a version of this and see how well it works.

[sannegrinovero]

thank you,

One possibility would be to inject the rule into the implementation method as normal, type check when it is first triggered and if we find the thrown exception is not declared by the implementation method ensure that an empty stub is executed instead of the rule. This means we still have the overhead of triggering the rule every time it gets called (i.e. executing the injected code which calls out to the rule execution engine) but none of the cost of executing the BIND IF or DO clauses. It's not perfect but it will give the desired behaviour

I got confused here. I'd expect the rule to be separately injected in all mathing implementations, separately. Are you saying that if I have to implementations A and B, having A omitting the "throws" clause, and B being fine, if Byteman happens to trigger first on A it will fail to inject the code on B?

I'd expect it to try again because A.class!=B.class, so my expectation is that it's a different trigger event? If it's trying again why should you inject an empty stub to be able to trigger again? Of course the class definition won't change before the next invocation.

[adinn]

Sanne Grinovero wrote:

I got confused here. I'd expect the rule to be separately injected in all mathing implementations, separately. Are you saying that if I have to implementations A and B, having A omitting the "throws" clause, and B being fine, if Byteman happens to trigger first on A it will fail to inject the code on B?

I'd expect it to try again because A.class!=B.class, so my expectation is that it's a different trigger event? If it's trying again why should you inject an empty stub to be able to trigger again? Of course the class definition won't change before the next invocation.

No, well get unconfused then as what you say here is correct. Let me explain it again.

Assume we have an interface rule for interface I method m throws E and the DO clause ends in THROW new E(). Let's say we we inject into class A implements I. We cannot reliably check whether A.m throws E at injection time because E may not be loaded yet (that's just a nasty feature of how the java.lang.instrumwenrt code works). Well ok, we might be able to match then name in the THROW expression with the name in the method signature but we can't rely on that because the THROW expression might refer to E1 extends E and we cannot check that without being sure that E1 and E are both loaded.

So, we typecheck the first time the rule is triggered (by which time  E will be loaded). If we find that A.m throws E then, assuming the rest of the rule  is ok we can execute the rule without a problem. If we find that A.m does not throw E then we cannot safely execute the rule. Currently we record a typecheck error and disable execution of the rule. Instead we need to record a  typecheck warning and disable execution.

We cannot uninstall the rule in order to undo the injection. This is because we might have another class B implements I where B.m throws A and we still  want the ruel to apply for B.So, we have to leave the code injected into A.m but stop it actually executing anything. Disabling execution is not difficult to do. We already do it when a rule fails to typecheck.