On Mon, 25 Nov 2024 17:38:55 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
> To the feature itself, it seems pretty complicated and invasive As for complexity, the feature consists of: * A Java agent `OpenFilesAgent`, containing 277 source code lines with: * A `main` method to package itself as a JAR for use with `-javaagent` on the test's command line * A `premain` method to add the `ClassFileTransformer`, make the runtime class `OpenFiles` available for the JDK's boot class loader and retransform any classes if needed. * The class file transformer itself, injecting hooks at around open and close operations in `FileInputStream`, `FileOutputStream`, `RandomAccessFile` and `sun.nio.ch.FileChannelImpl`. This uses the `java.lang.classfile` API. * An API support class `OpenFiles`, containing 79 source code lines which: * Allows instumented code to register a file as being opened or closed. * Provides an API for tests to assert files as open or closed * A test for the implementation the the feature itself, verifying that the feature works as expected across a wide range of JDK file primitives. To make use of the feature, tests must: * Add a `@run driver` tag to prepare the agent JAR files * Run in `/othervm` * Add the `-javaagent:OpenFilesAgent.jar` JVM option * Import the `OpenFiles` API * Add `assertOpen`, `assertClosed` calls as required by the test logic * Remove / clean up any old assertion mechanisms (if so desired) Yes, the feature has a non-negligble maintenence cost. However, given the potential benefits of cathing bugs early, I think the implementation size or compexity is not too bad considering what is offered. Byte code instrumentation is never pretty, but with the new `ClassFile` API I think the resulting code was relatively straightforward. (Although I have used ASM a lot in the past, this was my first experience using the ClassFile API. I'm sure my code can be improved). And with the `ClassFile` API we can hopefully expect more maintainers to be experienced with class file transformations in the future. ------------- PR Comment: https://git.openjdk.org/jdk/pull/22343#issuecomment-2498818742
