#880 Fix ResultSet move incorrectly closes input clob

Author: mrotteveel

src/docs/asciidoc/release_notes.adoc

@@ -35,12 +35,13 @@ For known issues, consult <<known-issues>>.
 
 === Jaybird 6.0.3
 
-The following was fixed or changed since Jaybird 6.0.3:
+The following was fixed or changed since Jaybird 6.0.2:
 
 * Fixed: statement close could cause a hang of the connection (https://github.com/FirebirdSQL/jaybird/issues/876[#876])
 +
 A large amount (>= 64) of statement closes (or cleanup of leaked statements) without interleaving other server operations could cause a connection to Firebird 4.0 or higher to hang.
 This could also hang the cleaner thread used for closing leaked statements.
+* Fixed: `ResultSet` move incorrectly closes input `Clob` (https://github.com/FirebirdSQL/jaybird/issues/880[#880])
 
 === Jaybird 6.0.2
 

src/main/org/firebirdsql/jdbc/FBBlob.java

@@ -271,10 +271,14 @@ public boolean isSegmented() throws SQLException {
     }
 
     @Override
-    public FirebirdBlob detach() throws SQLException {
-        try (LockCloseable ignored = withLock()) {
+    public FBBlob detach() throws SQLException {
+        try (var ignored = withLock()) {
             checkClosed();
-            return new FBBlob(gdsHelper, blobId, blobListener, config);
+            final var blobCopy = new FBBlob(gdsHelper, blobId, blobListener, config);
+            if (cachedInlineBlob != null) {
+                blobCopy.cachedInlineBlob = cachedInlineBlob.copy();
+            }
+            return blobCopy;
         }
     }
 

src/main/org/firebirdsql/jdbc/field/FBCachedLongVarCharField.java

@@ -45,15 +45,20 @@ final class FBCachedLongVarCharField extends FBLongVarCharField {
     }
 
     @Override
-    public Blob getBlob() throws SQLException {
-        if (isNull()) return null;
-        return new FBCachedBlob(getFieldData());
+    public Blob getBlob() {
+        return getBlobInternal();
     }
 
-    @SuppressWarnings("DataFlowIssue")
     @Override
-    public Clob getClob() throws SQLException {
-    	if (isNull()) return null;
-    	return new FBCachedClob((FBCachedBlob) getBlob(), blobConfig);
+    protected FBCachedBlob getBlobInternal() {
+        final byte[] fieldData = getFieldData();
+        return fieldData != null ? new FBCachedBlob(fieldData) : null;
     }
+
+    @Override
+    public Clob getClob() {
+        final FBCachedBlob blob = getBlobInternal();
+        return blob != null ? new FBCachedClob(blob, blobConfig) : null;
+    }
+
 }

src/main/org/firebirdsql/jdbc/field/FBLongVarCharField.java

@@ -19,7 +19,9 @@
 package org.firebirdsql.jdbc.field;
 
 import org.firebirdsql.gds.impl.GDSHelper;
+import org.firebirdsql.gds.ng.FbTransaction;
 import org.firebirdsql.gds.ng.fields.FieldDescriptor;
+import org.firebirdsql.gds.ng.listeners.TransactionListener;
 import org.firebirdsql.jaybird.props.PropertyConstants;
 import org.firebirdsql.jdbc.FBBlob;
 import org.firebirdsql.jdbc.FBClob;
@@ -94,28 +96,34 @@ public void close() throws SQLException {
 
     @Override
     public Blob getBlob() throws SQLException {
+        final FirebirdBlob blob = getBlobInternal();
+        return blob != null ? registerWithTransaction(blob.detach()) : null;
+    }
+
+    protected FirebirdBlob getBlobInternal() {
         if (blob != null) return blob;
-        byte[] bytes = getFieldData();
+        final byte[] bytes = getFieldData();
         if (bytes == null) return null;
 
         return blob = new FBBlob(gdsHelper, getDatatypeCoder().decodeLong(bytes), blobListener, blobConfig);
     }
 
     @Override
     public Clob getClob() throws SQLException {
-    	FBBlob blob = (FBBlob) getBlob();
-        return blob != null ? new FBClob(blob) : null;
+    	final FBBlob blob = (FBBlob) getBlobInternal();
+        if (blob == null) return null;
+        return new FBClob(registerWithTransaction(blob.detach()));
     }
 
     @Override
     public InputStream getBinaryStream() throws SQLException {
-        Blob blob = getBlob();
+        final Blob blob = getBlobInternal();
         return blob != null ? blob.getBinaryStream() : null;
     }
 
     @Override
     public byte[] getBytes() throws SQLException {
-        final FirebirdBlob blob = (FirebirdBlob) getBlob();
+        final FirebirdBlob blob = getBlobInternal();
         return blob != null ? blob.getBytes() : null;
     }
 
@@ -288,4 +296,15 @@ private void copyBytes(byte @NonNull [] bytes, int length) throws SQLException {
         blobExplicitNull = false;
     }
 
+    @NullMarked
+    private <T extends FirebirdBlob> T registerWithTransaction(T blob) {
+        if (blob instanceof TransactionListener transactionListener) {
+            FbTransaction currentTransaction = gdsHelper.getCurrentTransaction();
+            if (currentTransaction != null) {
+                currentTransaction.addWeakTransactionListener(transactionListener);
+            }
+        }
+        return blob;
+    }
+
 }

src/test/org/firebirdsql/jdbc/FBResultSetTest.java

@@ -30,6 +30,7 @@
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.StringReader;
 import java.nio.charset.StandardCharsets;
@@ -527,7 +528,7 @@ void testUpdatableResultSet(String scrollableCursorPropertyValue) throws Excepti
                         counter++;
                     }
 
-                    assertEquals(counter - 1, recordCount, "Should process " + recordCount + " rows");
+                    assertEquals(recordCount, counter - 1, "Should process " + recordCount + " rows");
 
                     // check the insertRow() feature
                     int newId = recordCount + 1;
@@ -1445,6 +1446,7 @@ void testRequiringMultipleFetches() throws Exception {
      */
     @ParameterizedTest
     @MethodSource
+    @SuppressWarnings("MagicConstant")
     void testIsBeforeFirst_isAfterLast_emptyResultSet_bug807(int resultSetType, int resultSetConcurrency,
             String scrollableCursorPropertyValue) throws SQLException {
         try (var connection = createConnection(scrollableCursorPropertyValue)) {
@@ -1493,6 +1495,7 @@ void testNextAfterLastRowShouldNotCloseResultSet(boolean autoCommit) throws Exce
 
     @ParameterizedTest
     @MethodSource
+    @SuppressWarnings("MagicConstant")
     void wasNull_onInsertRow(int resultSetType, String scrollableCursorPropertyValue) throws Exception {
         try (var connection = createConnection(scrollableCursorPropertyValue)) {
             executeCreateTable(connection, CREATE_TABLE_STATEMENT);
@@ -1801,6 +1804,37 @@ void updateObject_String_InputStream_type_scaleOrLength() throws SQLException {
         }
     }
 
+    @Test
+    void clobRemainsOpenAfterNext() throws Exception {
+        try (var connection = getConnectionViaDriverManager()) {
+            executeCreateTable(connection, CREATE_TABLE_STATEMENT);
+            connection.setAutoCommit(false);
+            createTestData(2, i -> "clob-value-" + i, connection, "blob_str");
+            connection.commit();
+
+            Clob secondClob;
+
+            try (var stmt = connection.createStatement()) {
+                var rs = stmt.executeQuery("select id, blob_str from test_table order by id");
+                assertNextRow(rs);
+                Clob firstClob = rs.getClob(2);
+                assertNextRow(rs);
+                try (BufferedReader firstReader = new BufferedReader(
+                        assertDoesNotThrow(() -> firstClob.getCharacterStream(),
+                                "should be able to get character stream from clob after next"))) {
+                    assertEquals("clob-value-1", firstReader.readLine());
+                }
+                secondClob = rs.getClob(2);
+                assertNoNextRow(rs);
+            }
+            try (BufferedReader secondReader = new BufferedReader(
+                    assertDoesNotThrow(() -> secondClob.getCharacterStream(),
+                            "should be able to get character stream from clob after statement close"))) {
+                assertEquals("clob-value-2", secondReader.readLine());
+            }
+        }
+    }
+
     static Stream<String> scrollableCursorPropertyValues() {
         // We are unconditionally emitting SERVER, to check if the value behaves appropriately on versions that do
         // not support server-side scrollable cursors