RFR: 8344911: Introduce test utility for asserting file open status [v2]

Eirik Bjørsnøs eirbjo at openjdk.org
Tue Nov 26 14:41:38 UTC 2024


On Mon, 25 Nov 2024 17:19:41 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please review this PR which  adds a utility API in the test libraries to assert whether a file is currently open.
>> 
>> Several OpenJDK  tests currently rely on approximations to check this, including deletion (fails only on Windows), or checking `/proc/<pid>/fd` (Works only on Linux). These approximations are blunt instruments, it would be better to have an exact cross platform solution for this need to check if a file is open or closed.
>> 
>> See https://github.com/openjdk/jdk/pull/22343#issuecomment-2498571155 for details
>> 
>> Verification:
>> With `OpenFilesTest` temporarily in tier1, GHA completed successfully across platforms.
>
> Eirik Bjørsnøs has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   Test infrastructure to assert that a file is open or closed

Here's my take on future maintenance costs of the utility:

Instrumentation churn is unlikely:

* JDK-8054720 introduced Java wrappers around native `open` methods in `FileInputStream`, `FileOutputStream` and `RandomFileAccess`, specifically to ease instrumentation. These are unlikely to change very often, if at all, same reasoning applies to the `close` methods.
* The signature of `FileChannelImpl:open` was last changed in 2014, when  JDK-8033917 added the file path as a parameter. Before that, the signature did not change since the initial load.
* The `java.lang.classfile` API is exiting preview, so we can expect less churn in this area.

Some enhancement to the assertion API is likely:

* The initial assertion API is intentionally kept very limited, allowing only assertion of the open status of single files.
* A reasonable future enhancement would be to add support for running code in a block and asserting on the file actions occurring in the block as a whole. 
* One example could be `assertNoLeak` which takes a block of code and verifies that any files opened during the execution of the block is also closed in the block.
* Such APIs could also confine the scope of file access to the scope of the code block, preventing leaks in one test from effecting unrelated tests in the same JVM instance.

Feature creep is possible, but discouraged

* It is conceivable that maintainers would find it convenient to add instrumentation to this utility beyond file access, making t a more general tool for verifying various APIs. I'm not sure that's a good idea, such features should be evaluated and developed on their own merits and independently of this utility.     

Some bug fixing is likely

* The implementation test should verify most APIs / features, but yes, all code has bug. Assuming 1-25 bugs per 1000 lines of code, the implementation should have beween 0.3 and 9 bugs :-)

-------------

PR Comment: https://git.openjdk.org/jdk/pull/22343#issuecomment-2500976491


More information about the core-libs-dev mailing list