Can Java compiler optimisation cause Google Guava's Preconditions checks to be ignored?

Question

We have a POJO in our Java application let's say Request which has validateRequest() method that basically validates some of it's fields using Guava's Preconditions library.

So, Basically Class looks like this :

public class Request { 
   private Integer field1; 
   
   private String field2;
   .
   .
   private String fieldN;

   
   public String validateRequest() {
      try { 
           Preconditions.checkArgument( getField1() >= 0, "error message" );

           Preconditions.checkArgument( StringUtils.isNotBlank(getField2()), "error message" );
           .
           .
        } catch (IllegalArgumentException | NullPointerException e){
            //emit some metric
            return e.getMessage();
        }
        return null;
   }
}

We call the below method in all the flows after building the Request POJO.

public void someMethod(Request request) {
    String message = request.validateRequest();
    if(message!= null)
       throw new Exception(message);
}

In some of the flows in our application we are never initialising field1 and field2 , so they are always null in those flows. So, ideal expectation is that validateRequest should return error message for those flows. But, what we notice is production is that this application never returns error message. It is as if Preconditions are never executed! Even metric that we emit is zero. On searching web, found out that compiler's optimisation can cause certain validation/assertion statements to be ignored, but didn't get more details around it.

When we explicitly remote debug to our Production box and put break-point on precondition statements they get executed and error message is returned and metric is emitted.

When we deploy this same application is deployed on Stage, code behaves as expected and throws validation error and metric is emitted.

Want to understand if compiler optimisation is the culprit here, or if root cause could be something else. How to verify that compiler optimisation is causing Preconditions to be ignored?

Answer 1

This code seems to be a kind of Cargo Cult Programming. It’s not clear what you expect from the call to Preconditions.checkArgument(…), but whatever it is, it’s not there.

When you call

Preconditions.checkArgument( getField1() >= 0, "error message" );

The expression getField1() >= 0 is evaluated in your method. You’ve already done the check before even calling the checkArgument method. All, the method does, is throwing a new IllegalArgumentException if the argument is false, with the message you’ve provided, i.e.

public static void checkArgument(boolean expression, Object errorMessage) {
    if(!expression) {
        throw new IllegalArgumentException(String.valueOf(errorMessage));
    }
}

which you then catch, to extract the very error message, you have provided yourself.

catch (IllegalArgumentException | NullPointerException e){
    //emit some metric
    return e.getMessage();
}

So, you have created an inefficient way to do the same as

if(getField1() < 0) return "error message";

---

Since the evaluation of getField1() >= 0 happens in your method, before the checkArgument method is invoked, it will also fail with a NullPointerException instead of ever invoking the checkArgument method when getField1() returned null. This JVM generated NullPointerException will have an implementation specific message or no message at all.

This is not an optimization but just the way method invocations work. First, the arguments are evaluated, then, the method is invoked. If the argument evaluation throws an exception the method is not invoked.

It’s not clear how you could see this working with a getField1() returning null during debugging, as it is impossible to pass null to a method expecting a primitive type boolean. Maybe you are confusing it with the scenario of getField2() returning null.

Your second check uses the expression StringUtils.isNotBlank(getField2()) which handles the null case, as isNotBlank will return false when the argument is null.

You can fix the first check by handling the null case, e.g.

if(getField1() == null || getField1() < 0) return "error message";

or, if you insist on creating and catching an exception, just to extract the very message you have specified yourself,

Preconditions.checkArgument(getField1()!=null && getField1()>=0, "error message");