Update for to use Java 21

* update tests to run on 21 * remove deprecated uses of finalize, make DiskBackQueue and associated Autocloseable so that it's easier for downstream code to handle them without finalize Note: if this proves to be insufficient we should look into implementing a Cleaner * remove deprecated boxing constructors new Byte() -> Byte.valueof() etc
samtools · Dec 18, 2023 · 0895474 · 0895474
1 parent 2d13eb7
commit 0895474
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 60 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        java: [ 17 ]
+        java: [ 21 ]
         experimental: [false]
       fail-fast: false
     continue-on-error: ${{ matrix.experimental }}
@@ -49,10 +49,10 @@ jobs:
     name: Tests that require external APIs
     steps:
       - uses: actions/checkout@v3
-      - name: Set up java 17
+      - name: Set up java 21
         uses: actions/setup-java@v3
         with:
-          java-version: '17'
+          java-version: '21'
           distribution: 'adopt'
           cache: gradle
       - name: Grant execute permission for gradlew
@@ -75,10 +75,10 @@ jobs:
     name: FTP tests
     steps:
       - uses: actions/checkout@v3
-      - name: Set up java 17
+      - name: Set up java 21
         uses: actions/setup-java@v3
         with:
-          java-version: '17'
+          java-version: '21'
           distribution: 'adopt'
           cache: gradle
       - name: Grant execute permission for gradlew
@@ -100,10 +100,10 @@ jobs:
     name: SpotBugs
     steps:
       - uses: actions/checkout@v3
-      - name: Set up java 17
+      - name: Set up java 21
         uses: actions/setup-java@v3
         with:
-          java-version: '17'
+          java-version: '21'
           distribution: 'adopt'
           cache: gradle
       - name: Grant execute permission for gradlew

diff --git a/build.gradle b/build.gradle
@@ -47,7 +47,7 @@ dependencies {
 
 java {
     toolchain {
-        languageVersion = JavaLanguageVersion.of(17)
+        languageVersion = JavaLanguageVersion.of(21)
     }
     withJavadocJar()
     withSourcesJar()

diff --git a/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java b/src/main/java/htsjdk/samtools/util/DiskBackedQueue.java
@@ -54,7 +54,7 @@
  *
  * Created by bradt on 4/28/14.
  */
-public class DiskBackedQueue<E> implements Queue<E> {
+public class DiskBackedQueue<E> implements Queue<E>, AutoCloseable {
     private final int maxRecordsInRamQueue;
     private final Queue<E> ramRecords;
     private Path diskRecords = null;
@@ -95,7 +95,7 @@ private DiskBackedQueue(final SortingCollection.Codec<E> codec,
         this.tmpDirs = tmpDirs;
         this.codec = codec;
         this.maxRecordsInRamQueue = (maxRecordsInRam == 0) ? 0 : maxRecordsInRam - 1; // the first of our ram records is stored as headRecord
-        this.ramRecords = new ArrayDeque<E>(this.maxRecordsInRamQueue);
+        this.ramRecords = new ArrayDeque<>(this.maxRecordsInRamQueue);
     }
 
     /**
@@ -248,18 +248,6 @@ public void clear() {
         this.canAdd = true;
     }
 
-    /**
-     * Clean up disk resources in case clear() has not been explicitly called (as would be preferable)
-     * Closes the input and output streams associated with this DiskBackedQueue and deletes the temporary file
-     *
-     * @throws Throwable
-     */
-    @Override
-    protected void finalize() throws Throwable {
-        this.closeIOResources();
-        super.finalize(); // NB: intellij wanted me to do this. Need I?  I'm not extending anything
-    }
-
     /**
      * Write the present record to the end of a file representing the tail of the queue.
      * @throws RuntimeIOException
@@ -420,4 +408,13 @@ public Object[] toArray() {
     public <T1> T1[] toArray(final T1[] a) {
         throw new UnsupportedOperationException("DiskBackedQueue does not support toArray(T1[] a)");
     }
+
+    /**
+    * Clean up disk resources in case clear() has not been explicitly called (as would be preferable)
+    * Closes the input and output streams associated with this DiskBackedQueue and deletes the temporary file
+    */
+    @Override
+    public void close() {
+        clear();
+    }
 }
diff --git a/src/main/java/htsjdk/samtools/util/FileAppendStreamLRUCache.java b/src/main/java/htsjdk/samtools/util/FileAppendStreamLRUCache.java
@@ -56,7 +56,6 @@ public OutputStream makeValue(final File file) {
                 // In case the file could not be opened because of too many file handles, try to force
                 // file handles to be closed.
                 System.gc();
-                System.runFinalization();
                 try {
                     return IOUtil.maybeBufferOutputStream(new FileOutputStream(file, true));
                 }

diff --git a/src/main/java/htsjdk/samtools/util/SamRecordTrackingBuffer.java b/src/main/java/htsjdk/samtools/util/SamRecordTrackingBuffer.java
@@ -54,7 +54,7 @@
  *
  * @author bradtaylor
  */
-public class SamRecordTrackingBuffer<T extends SamRecordWithOrdinal> {
+public class SamRecordTrackingBuffer<T extends SamRecordWithOrdinal> implements AutoCloseable {
     private int availableRecordsInMemory; // how many more records can we store in memory
     private final int blockSize; // the size of each block
     private final List<File> tmpDirs; // the list of temporary directories to use
@@ -78,7 +78,7 @@ public SamRecordTrackingBuffer(final int maxRecordsInRam, final int blockSize, f
         this.tmpDirs = tmpDirs;
         this.queueHeadRecordIndex = -1;
         this.queueTailRecordIndex = -1;
-        this.blocks = new ArrayDeque<BufferBlock>();
+        this.blocks = new ArrayDeque<>();
         this.header = header;
         this.clazz = clazz;
     }
@@ -184,6 +184,7 @@ public void setResultState(final SamRecordWithOrdinal samRecordWithOrdinal, fina
     /**
      * Close IO resources associated with each underlying BufferBlock
      */
+    @Override
     public void close() {
         while (!blocks.isEmpty()) {
             final BufferBlock block = blocks.pollFirst();
@@ -194,7 +195,7 @@ public void close() {
     /**
      * This stores blocks of records, either in memory or on disk, or both!
      */
-    private class BufferBlock {
+    private class BufferBlock implements AutoCloseable {
         private final DiskBackedQueue<SAMRecord> recordsQueue;
         private final int maxBlockSize;
         private long currentStartIndex;
@@ -256,7 +257,7 @@ public void add(final SamRecordWithOrdinal samRecordWithOrdinal) {
             }
         }
 
-        private int ensureIndexFitsInAnInt(final long value) {
+        private static int ensureIndexFitsInAnInt(final long value) {
             if (value < Integer.MIN_VALUE || Integer.MAX_VALUE < value) throw new SAMException("Error: index out of range: " + value);
             return (int)value;
         }
@@ -288,7 +289,7 @@ public SamRecordWithOrdinal next() throws IllegalStateException {
             if (this.canEmit()) {
                 try {
                     // create a wrapped record for the head of the queue, and set the underlying record's examined information appropriately
-                    final SamRecordWithOrdinal samRecordWithOrdinal = clazz.newInstance();
+                    final SamRecordWithOrdinal samRecordWithOrdinal = clazz.getDeclaredConstructor().newInstance();
                     samRecordWithOrdinal.setRecord(this.recordsQueue.poll());
                     samRecordWithOrdinal.setRecordOrdinal(this.currentStartIndex);
                     samRecordWithOrdinal.setResultState(this.resultStateIndexes.get(ensureIndexFitsInAnInt(this.currentStartIndex - this.originalStartIndex)));
@@ -316,6 +317,15 @@ public SamRecordWithOrdinal next() throws IllegalStateException {
          * Close disk IO resources associated with the underlying records queue.
          * This must be called when a block is no longer needed in order to prevent memory leaks.
          */
-        public void clear() { this.recordsQueue.clear(); }
+        public void clear() { this.recordsQueue.close(); }
+
+        /**
+         * Close disk IO resources associated with the underlying records queue.
+         * This must be called when a block is no longer needed in order to prevent memory leaks.
+         */
+        @Override
+        public void close() {
+            clear();
+        }
     }
 }
diff --git a/src/test/java/htsjdk/samtools/SAMBinaryTagAndValueUnitTest.java b/src/test/java/htsjdk/samtools/SAMBinaryTagAndValueUnitTest.java
@@ -11,19 +11,19 @@ public class SAMBinaryTagAndValueUnitTest extends HtsjdkTest {
     @DataProvider(name="allowedAttributeTypes")
     public Object[][] allowedTypes() {
         return  new Object[][] {
-                {new String("a string")},
-                {new Byte((byte) 7)},
-                {new Short((short) 8)},
-                {new Integer(0)},
-                {new Character('C')},
-                {new Float(0.1F)},
+                {"a string"},
+                {Byte.valueOf((byte) 7)},
+                {Short.valueOf((short) 8)},
+                {Integer.valueOf(0)},
+                {Character.valueOf('C')},
+                {Float.valueOf(0.1F)},
                 // unsigned longs
-                {new Long(0)},
-                {new Long(BinaryCodec.MAX_UINT)},
+                {Long.valueOf(0)},
+                {Long.valueOf(BinaryCodec.MAX_UINT)},
                 // signed longs
-                {new Long(-1L)},
-                {new Long(Integer.MAX_VALUE)},
-                {new Long(Integer.MIN_VALUE)},
+                {Long.valueOf(-1L)},
+                {Long.valueOf(Integer.MAX_VALUE)},
+                {Long.valueOf(Integer.MIN_VALUE)},
                 // array values
                 {new byte[]{0, 1, 2}},
                 {new short[]{3, 4, 5}},
@@ -45,9 +45,9 @@ public void test_isAllowedConstructor(final Object value) {
     @DataProvider(name="notAllowedAttributeTypes")
     public Object[][] notAllowedTypes() {
         return  new Object[][] {
-                {new Long(BinaryCodec.MAX_UINT + 1L)},
-                {new Long(Integer.MIN_VALUE - 1L)},
-                {new Double(0.3F)},
+                {Long.valueOf(BinaryCodec.MAX_UINT + 1L)},
+                {Long.valueOf(Integer.MIN_VALUE - 1L)},
+                {Double.valueOf(0.3F)},
                 {new Object()},
                 {new Object[]{}},
                 {new Integer[]{}}
@@ -76,7 +76,7 @@ public Object[][] allowedUnsignedArrayTypes() {
     @Test(dataProvider="allowedUnsignedArrayTypes")
     public void test_isAllowedUnsignedArrayAttribute(final Object value) {
         final short binaryTag = SAMTag.makeBinaryTag("UI");
-        Assert.assertNotNull(new SAMBinaryTagAndUnsignedArrayValue(binaryTag, value));
+        new SAMBinaryTagAndUnsignedArrayValue(binaryTag, value);
     }
 
     @DataProvider(name="notAllowedUnsignedArrayTypes")
@@ -97,29 +97,29 @@ public void test_isNotAllowedUnsignedArrayAttribute(final Object value) {
     public Object[][] hashCopyEquals() {
         final short tag = SAMTag.makeBinaryTag("UI");
         return new Object[][] {
-                {new SAMBinaryTagAndValue(tag, new String("a string")), new SAMBinaryTagAndValue(tag, new String("a string")), true, true},
-                {new SAMBinaryTagAndValue(tag, new String("a string")), new SAMBinaryTagAndValue(tag, new String("different string")), false, false},
+                {new SAMBinaryTagAndValue(tag, "a string"), new SAMBinaryTagAndValue(tag, "a string"), true, true},
+                {new SAMBinaryTagAndValue(tag, "a string"), new SAMBinaryTagAndValue(tag, "different string"), false, false},
 
-                {new SAMBinaryTagAndValue(tag, new Byte((byte) 0)), new SAMBinaryTagAndValue(tag, new Byte((byte) 0)), true, true},
-                {new SAMBinaryTagAndValue(tag, new Byte((byte) 0)), new SAMBinaryTagAndValue(tag, new Byte((byte) 1)), false, false},
+                {new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 0)), new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 0)), true, true},
+                {new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 0)), new SAMBinaryTagAndValue(tag, Byte.valueOf((byte) 1)), false, false},
 
-                {new SAMBinaryTagAndValue(tag, new Short((short) 0)), new SAMBinaryTagAndValue(tag, new Short((short) 0)), true, true},
-                {new SAMBinaryTagAndValue(tag, new Short((short) 0)), new SAMBinaryTagAndValue(tag, new Short((short) 1)), false, false},
+                {new SAMBinaryTagAndValue(tag, Short.valueOf((short) 0)), new SAMBinaryTagAndValue(tag, Short.valueOf((short) 0)), true, true},
+                {new SAMBinaryTagAndValue(tag, Short.valueOf((short) 0)), new SAMBinaryTagAndValue(tag, Short.valueOf((short) 1)), false, false},
 
-                {new SAMBinaryTagAndValue(tag, new Integer(0)), new SAMBinaryTagAndValue(tag, new Integer(0)), true, true},
-                {new SAMBinaryTagAndValue(tag, new Integer(0)), new SAMBinaryTagAndValue(tag, new Integer(0)), true, true},
+                {new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), true, true},
+                {new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), new SAMBinaryTagAndValue(tag, Integer.valueOf(0)), true, true},
 
-                {new SAMBinaryTagAndValue(tag, new Character('C')), new SAMBinaryTagAndValue(tag, new Character('C')), true, true},
-                {new SAMBinaryTagAndValue(tag, new Character('C')), new SAMBinaryTagAndValue(tag, new Character('D')), false, false},
+                {new SAMBinaryTagAndValue(tag, Character.valueOf('C')), new SAMBinaryTagAndValue(tag, Character.valueOf('C')), true, true},
+                {new SAMBinaryTagAndValue(tag, Character.valueOf('C')), new SAMBinaryTagAndValue(tag, Character.valueOf('D')), false, false},
 
-                {new SAMBinaryTagAndValue(tag,new Float(0.1F)), new SAMBinaryTagAndValue(tag, new Float(0.1F)), true, true},
-                {new SAMBinaryTagAndValue(tag, new Float(0.1F)), new SAMBinaryTagAndValue(tag, new Float(0.2F)), false, false},
+                {new SAMBinaryTagAndValue(tag,Float.valueOf(0.1F)), new SAMBinaryTagAndValue(tag, Float.valueOf(0.1F)), true, true},
+                {new SAMBinaryTagAndValue(tag, Float.valueOf(0.1F)), new SAMBinaryTagAndValue(tag, Float.valueOf(0.2F)), false, false},
 
-                {new SAMBinaryTagAndValue(tag,new Long(37L)), new SAMBinaryTagAndValue(tag, new Long(37L)), true, true},
-                {new SAMBinaryTagAndValue(tag, new Long(37L)), new SAMBinaryTagAndValue(tag, new Long(38L)), false, false},
+                {new SAMBinaryTagAndValue(tag,Long.valueOf(37L)), new SAMBinaryTagAndValue(tag, Long.valueOf(37L)), true, true},
+                {new SAMBinaryTagAndValue(tag, Long.valueOf(37L)), new SAMBinaryTagAndValue(tag, Long.valueOf(38L)), false, false},
 
-                {new SAMBinaryTagAndValue(tag,new Long(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, new Long(BinaryCodec.MAX_UINT)), true, true},
-                {new SAMBinaryTagAndValue(tag, new Long(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, new Long(BinaryCodec.MAX_UINT-1)), false, false},
+                {new SAMBinaryTagAndValue(tag,Long.valueOf(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, Long.valueOf(BinaryCodec.MAX_UINT)), true, true},
+                {new SAMBinaryTagAndValue(tag, Long.valueOf(BinaryCodec.MAX_UINT)), new SAMBinaryTagAndValue(tag, Long.valueOf(BinaryCodec.MAX_UINT-1)), false, false},
 
                 // arrays