Set Java 8 release compiler option

[ryandens]

Use --release flag to target JDK 8 bytecode compatibility

My recent change to capture HTTP requests contained a fascinating bug resulting in the bug-fix not working when running on JVMs <= 1.8 due to a NoSuchMethodError being thrown when this line executes. This was incredibly surprising to me, because the position(int) API exists on Buffer (the superclass of ByteBuffer ) in java 1.8, so why do we have problems here? Confused, I did a quick google search and found an incredibly helpful article right away: ByteBuffer and the Dreaded NoSuchMethodError. The tl;dr of this change is that, the bytecode we produce prior to this change looked like this:

BEFORE

  public static void handleRead(java.nio.ByteBuffer, int, org.hypertrace.agent.core.instrumentation.SpanAndBuffer);
    descriptor: (Ljava/nio/ByteBuffer;ILorg/hypertrace/agent/core/instrumentation/SpanAndBuffer;)V
    flags: (0x0009) ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=6, args_size=3
         0: iload_1
         1: ifgt          5
         4: return
         5: aload_0
         6: aload_0
         7: invokevirtual #23                 // Method java/nio/ByteBuffer.position:()I
        10: iload_1
        11: isub
        12: invokevirtual #24                 // Method java/nio/ByteBuffer.position:(I)Ljava/nio/ByteBuffer;

AFTER

 public static void handleRead(java.nio.ByteBuffer, int, org.hypertrace.agent.core.instrumentation.SpanAndBuffer);
    descriptor: (Ljava/nio/ByteBuffer;ILorg/hypertrace/agent/core/instrumentation/SpanAndBuffer;)V
    flags: (0x0009) ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=6, args_size=3
         0: iload_1
         1: ifgt          5
         4: return
         5: aload_0
         6: aload_0
         7: invokevirtual #23                 // Method java/nio/ByteBuffer.position:()I
        10: iload_1
        11: isub
        12: invokevirtual #24                 // Method java/nio/ByteBuffer.position:(I)Ljava/nio/Buffer;

Spot the difference? The bytecode for this snippet is identical, but when compiled without the --release 8, the invokevirtual #24 call refers to a method reference in the constant pool with a signature matching the descriptor Method java/nio/ByteBuffer.position:(I)Ljava/nio/ByteBuffer. However, when we compile it with the --release 8 flag the method reference in the constant pool is Method java/nio/ByteBuffer.position:(I)Ljava/nio/Buffer. This resolves the cross-jre compatibility issue of using the ByteBuffer.position(int) API because Method java/nio/ByteBuffer.position:(I)Ljava/nio/ByteBuffer does not exist on JDK 1.8 or below.

Other notes

It's worth noting that this has impacted many well-known open source projects. The blog i linked above links to several other projects that encountered this exact issue:

• Eclipse Jetty
• Apache Pulsar
• Eclipse Vert.x
• Apache Thrift
• the Yugabyte DB client
• Debezium (the project the author found the bug on)

The author also evaluated other methods of resolving this issue and verifying that it couldn't happen again, and determined the --release flag was the best option:

Note that theoretically you could achieve the same effect also when using --source and --target; by means of the --boot-class-path option, you could advise the compiler to use a specific set of bootstrap class files instead of those from the JDK used for compilation. But that’d be quite a bit more cumbersome as it requires you to actually provide the classes of the targeted Java version, whereas --release will make use of the signature data coming with the currently used JDK itself.

[ryandens]

we should start testing against JVM 8 and 11. Can you file a ticket for it?

Agreed, but this is definitely an interesting one as this code compiles and works properly when compiled and run on 1.8, just not when it was compiled with 11 and run on 1.8. I think we'd want to leverage gradle's support for toolchains to register a test task that uses a different jvm to run the tests than the one used to compile the instrumentation module. Did you have an idea of how this might look?

[pavolloffay]

We should be building with Java 11 and using --release=8 as this PR does. Then define matrix build and test against all supported LTS versions (8, 11) and the latest release (16).

[ryandens]

Yeah that makes sense, I'm just curious if you had any other ideas for how that might be implemented in practice. The simplest option (to me) would seem to be using the GitHub actions setup-java matrix to run the tests in the instrumentation module across multiple JREs after compiling on 11. However, when running the test task on a different JDK, Gradle would detect changes and recompile, making it difficult to detect niche bugs like this one. How are you envisioning coercing Gradle to compile with one JDK and run tests with a different JDK?

[ryandens]

Moving the conversation about cross JRE testing to #335