JDBC-587 Use of Blob can throw NullPointerException after commit

Restrict lifetime of blob to transaction (excluding cached blobs), and
ensure freed blobs throw a SQLException

src/documentation/release_notes.md

@@ -237,10 +237,22 @@ Changes in Jaybird 3.0.7
 
 The following has been changed or fixed since Jaybird 3.0.6
 
+-   Fixed: attempts to use a blob after it was freed or after transaction end
+    could throw a `NullPointerException` or just work depending on whether the
+    connection had a new transaction. ([JDBC-587](http://tracker.firebirdsql.org/browse/JDBC-587)) \
+    The lifetime of a blob is now restricted to the transaction that created it,
+    or - for blobs created using `Connection.createBlob()` - to the transaction
+    that populated it. Attempts to use the blob after the transaction ends will 
+    now throw a `SQLException` with message _"Blob is invalid. Blob was freed, 
+    or closed by transaction end."_ This restriction does not apply to cached 
+    blobs, see also next item. 
 -   Fixed: Instances of `java.sql.Blob` and `java.sql.Clob` obtained from a 
     result set were freed after calls to `ResultSet.next()`. ([JDBC-588](http://tracker.firebirdsql.org/browse/JDBC-588)) \
-    They will now remain valid until transaction end. You will need to call
-    `Blob.free()` if you want to invalidate them earlier.
+    Normal blobs will now remain valid until transaction end. You will need to 
+    call `Blob.free()` if you want to invalidate them earlier. \
+    Cached blobs (in auto-commit, holdable or scrollable result sets) will not
+    be automatically freed at transaction end. You will need to explicitly call
+    `Blob.free()` or rely on the garbage collector.
 
 ### Known issues in Jaybird 3.0.7
 

src/jna-client/org/firebirdsql/gds/ng/jna/JnaBlob.java

@@ -238,28 +238,29 @@ public byte[] getBlobInfo(byte[] requestItems, int bufferLength) throws SQLExcep
 
     @Override
     protected void closeImpl() throws SQLException {
-        synchronized (getSynchronizationObject()) {
-            try {
-                clientLibrary.isc_close_blob(statusVector, getJnaHandle());
-                processStatusVector();
-            } finally {
-                byteBuffer = null;
-            }
+        try {
+            clientLibrary.isc_close_blob(statusVector, getJnaHandle());
+            processStatusVector();
+        } finally {
+            releaseResources();
         }
     }
 
     @Override
     protected void cancelImpl() throws SQLException {
-        synchronized (getSynchronizationObject()) {
-            try {
-                clientLibrary.isc_cancel_blob(statusVector, getJnaHandle());
-                processStatusVector();
-            } finally {
-                byteBuffer = null;
-            }
+        try {
+            clientLibrary.isc_cancel_blob(statusVector, getJnaHandle());
+            processStatusVector();
+        } finally {
+            releaseResources();
         }
     }
 
+    @Override
+    protected void releaseResources() {
+        byteBuffer = null;
+    }
+
     private void processStatusVector() throws SQLException {
         getDatabase().processStatusVector(statusVector, null);
     }

src/main/org/firebirdsql/gds/JaybirdErrorCodes.java

@@ -73,6 +73,8 @@ public interface JaybirdErrorCodes {
     int jb_dbCryptCallbackInitError = 337248284;
     int jb_dbCryptDataError         = 337248285;
     int jb_hashAlgorithmNotAvailable                     = 337248286;
+    // Error codes backported from Jaybird 4 as needed
+    int jb_blobClosed               = 337248295;
 
     @SuppressWarnings("unused")
     int jb_range_end                = 337264639;

src/main/org/firebirdsql/gds/ng/AbstractFbBlob.java

@@ -125,10 +125,14 @@ public final void close() throws SQLException {
         try {
             synchronized (getSynchronizationObject()) {
                 if (!isOpen()) return;
-                checkDatabaseAttached();
-                checkTransactionActive();
                 try {
-                    closeImpl();
+                    if (isEndingTransaction()) {
+                        releaseResources();
+                    } else {
+                        checkDatabaseAttached();
+                        checkTransactionActive();
+                        closeImpl();
+                    }
                 } finally {
                     setOpen(false);
                 }
@@ -151,10 +155,14 @@ public final void close() throws SQLException {
     public final void cancel() throws SQLException {
         try {
             synchronized (getSynchronizationObject()) {
-                checkDatabaseAttached();
-                checkTransactionActive();
                 try {
-                    cancelImpl();
+                    if (isEndingTransaction()) {
+                        releaseResources();
+                    } else {
+                        checkDatabaseAttached();
+                        checkTransactionActive();
+                        cancelImpl();
+                    }
                 } finally {
                     setOpen(false);
                 }
@@ -171,6 +179,11 @@ public final void cancel() throws SQLException {
      */
     protected abstract void cancelImpl() throws SQLException;
 
+    /**
+     * Release Java resources held. This should not communicate with the Firebird server.
+     */
+    protected abstract void releaseResources();
+
     @Override
     public final Object getSynchronizationObject() {
         return syncObject;
@@ -184,11 +197,20 @@ public void transactionStateChanged(FbTransaction transaction, TransactionState
             return;
         }
         switch (newState) {
+        case COMMITTING:
+        case ROLLING_BACK:
+        case PREPARING:
+            try {
+                close();
+            } catch (SQLException e) {
+                log.error("Exception while closing blob during transaction end", e);
+            }
         case COMMITTED:
         case ROLLED_BACK:
             synchronized (getSynchronizationObject()) {
                 clearTransaction();
                 setOpen(false);
+                releaseResources();
             }
             break;
         default:
@@ -223,6 +245,7 @@ public void detached(FbDatabase database) {
                 open = false;
                 clearDatabase();
                 clearTransaction();
+                releaseResources();
             }
         }
         database.removeDatabaseListener(this);
@@ -233,6 +256,20 @@ public void warningReceived(FbDatabase database, SQLWarning warning) {
         // Do nothing
     }
 
+    /**
+     * @return {@code true} if the transaction is committing, rolling back or preparing
+     */
+    protected final boolean isEndingTransaction() {
+        FbTransaction transaction = getTransaction();
+        if (transaction != null) {
+            TransactionState transactionState = transaction.getState();
+            return transactionState == TransactionState.COMMITTING
+                    || transactionState == TransactionState.ROLLING_BACK
+                    || transactionState == TransactionState.PREPARING;
+        }
+        return false;
+    }
+
     /**
      * @throws java.sql.SQLException
      *         When no transaction is set, or the transaction state is not {@link TransactionState#ACTIVE}

src/main/org/firebirdsql/gds/ng/wire/AbstractFbWireBlob.java

@@ -83,12 +83,25 @@ protected void releaseBlob(int releaseOperation) throws SQLException {
 
     @Override
     protected void closeImpl() throws SQLException {
-        releaseBlob(WireProtocolConstants.op_close_blob);
+        try {
+            releaseBlob(WireProtocolConstants.op_close_blob);
+        } finally {
+            releaseResources();
+        }
     }
 
     @Override
     protected void cancelImpl() throws SQLException {
-        releaseBlob(WireProtocolConstants.op_cancel_blob);
+        try {
+            releaseBlob(WireProtocolConstants.op_cancel_blob);
+        } finally {
+            releaseResources();
+        }
+    }
+
+    @Override
+    protected void releaseResources() {
+        // Nothing to release
     }
 
     // NOTE If we need to override some of the blob operations below in the future, consider introducing a separate