RFR: 8347840: Fix testlibrary compilation warnings [v3]

Leonid Mesnik lmesnik at openjdk.org
Thu Jan 16 18:18:16 UTC 2025


On Thu, 16 Jan 2025 05:31:44 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   revert change
>
> test/lib/jdk/test/lib/format/ArrayDiff.java line 110:
> 
>> 108:      * @return an ArrayDiff instance for the two arrays and formatting parameters provided
>> 109:      */
>> 110:     @SuppressWarnings({"rawtypes", "unchecked"})
> 
> Just wondering where the unchecked warning arises in the code?

It is in the line 123.

> test/lib/jdk/test/lib/hprof/model/JavaHeapObject.java line 42:
> 
>> 40:  *
>> 41:  * @author      Bill Foote
>> 42:  */
> 
> I would suggest deleting any comment blocks that just have the @author tag, and deleting the @author elsewhere. All these files (lib/hprof) already have an author attribution comment.

done

> test/lib/jdk/test/lib/hprof/parser/ReadBuffer.java line 46:
> 
>> 44:     public int   getInt(long pos) throws IOException;
>> 45:     public long  getLong(long pos) throws IOException;
>> 46:     public void  close() throws IOException;
> 
> Why was this redefined to throw IOException rather than just Exception?

The javac complains about potential InterruptedException, so I changed type.
See
https://docs.oracle.com/javase/8/docs/api/java/lang/AutoCloseable.html
`Implementers of this interface are also strongly advised to not have the close method throw [InterruptedException](https://docs.oracle.com/javase/8/docs/api/java/lang/InterruptedException.html). This exception interacts with a thread's interrupted status, and runtime misbehavior is likely to occur if an InterruptedException is [suppressed](https://docs.oracle.com/javase/8/docs/api/java/lang/Throwable.html#addSuppressed-java.lang.Throwable-). More generally, if it would cause problems for an exception to be suppressed, the AutoCloseable.close method should not throw it.`

> test/lib/jdk/test/lib/hprof/parser/Reader.java line 96:
> 
>> 94:             } else if ((i >>> 8) == GZIP_HEADER_MAGIC) {
>> 95:                 // Possible gziped file, try decompress it and get the stack trace.
>> 96:                 String deCompressedFile = "heapdump" + System.currentTimeMillis() + ".hprof";
> 
> It is not obvious to me that there may not be a reason for closing all of the streams before opening new ones below.

Thanks for catching this. I missed that we try to re-open the same fail in else branch.

> test/lib/jdk/test/lib/thread/VThreadRunner.java line 100:
> 
>> 98:             if ((X) ex == ex) {
>> 99:                 throw (X) ex;
>> 100:             }
> 
> This doesn't make sense to me.

Sorry for mess, this shouldn't have been committed at all, just some drafting.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1918956737
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1918960995
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1918969394
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1918976104
PR Review Comment: https://git.openjdk.org/jdk/pull/23143#discussion_r1918981965


More information about the net-dev mailing list