#876 Fix hang when closing lots of statements

Author: mrotteveel

src/docs/asciidoc/release_notes.adoc

@@ -37,7 +37,10 @@ For known issues, consult <<known-issues>>.
 
 The following was fixed or changed since Jaybird 6.0.3:
 
-* ...
+* 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.
 
 === Jaybird 6.0.2
 

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

@@ -42,6 +42,7 @@
 import java.sql.SQLException;
 import java.util.Map;
 import java.util.WeakHashMap;
+import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
 import java.util.function.Function;
 
 import static java.util.Objects.requireNonNull;
@@ -253,54 +254,77 @@ private void deferredExceptionHandler(Exception exception) {
         }
     }
 
+    @SuppressWarnings("resource")
     private static final class CleanupAction implements Runnable, StatementListener, DatabaseListener {
 
+        private static final AtomicReferenceFieldUpdater<CleanupAction, FbWireDatabase> databaseUpdater =
+                AtomicReferenceFieldUpdater.newUpdater(CleanupAction.class, FbWireDatabase.class, "database");
+
+        private static final DeferredAction CLEANUP_FREE_DEFERRED_ACTION = new DeferredAction() {
+            @Override
+            public void processResponse(Response response) {
+                // nothing to do
+            }
+
+            @Override
+            public boolean requiresSync() {
+                return true;
+            }
+        };
+
         private final int handle;
-        @SuppressWarnings("java:S3077")
         private volatile FbWireDatabase database;
 
         private CleanupAction(AbstractFbWireStatement statement) {
             // NOTE: Care should be taken not to retain a handle to statement itself here
             handle = statement.getHandle();
-            database = statement.getDatabase();
+            FbWireDatabase database = statement.getDatabase();
+            databaseUpdater.set(this, database);
             database.addWeakDatabaseListener(this);
             statement.addWeakStatementListener(this);
         }
 
         @Override
         public void statementStateChanged(FbStatement sender, StatementState newState, StatementState previousState) {
             if (newState == StatementState.CLOSING) {
-                FbDatabase database = this.database;
-                if (database != null) {
-                    release(database);
-                }
+                releaseDatabaseReference();
                 sender.removeStatementListener(this);
             }
         }
 
         @Override
         public void detaching(FbDatabase database) {
-            release(database);
+            releaseDatabaseReference();
         }
 
-        private void release(FbDatabase database) {
-            this.database = null;
-            database.removeDatabaseListener(this);
+        private void releaseDatabaseReference() {
+            FbWireDatabase database = databaseUpdater.getAndSet(this, null);
+            if (database != null) {
+                database.removeDatabaseListener(this);
+            }
+        }
+
+        private FbWireDatabase releaseAndGetDatabaseReference() {
+            FbWireDatabase database = databaseUpdater.getAndSet(this, null);
+            if (database != null) {
+                database.removeDatabaseListener(this);
+            }
+            return database;
         }
 
         @Override
         public void run() {
-            FbWireDatabase database = this.database;
+            FbWireDatabase database = releaseAndGetDatabaseReference();
             if (database == null) return;
-            release(database);
             try (LockCloseable ignored = database.withLock()) {
                 if (!database.isAttached()) return;
                 XdrOutputStream xdrOut = database.getXdrStreamAccess().getXdrOut();
                 xdrOut.writeInt(WireProtocolConstants.op_free_statement); // p_operation
                 xdrOut.writeInt(handle); // p_sqlfree_statement
                 xdrOut.writeInt(ISCConstants.DSQL_drop); // p_sqlfree_option
                 xdrOut.flush();
-                database.enqueueDeferredAction(DeferredAction.NO_OP_INSTANCE);
+                // TODO: This may process deferred actions on the cleaner thread, we may want to change that
+                database.enqueueDeferredAction(CLEANUP_FREE_DEFERRED_ACTION);
             } catch (SQLException | IOException e) {
                 System.getLogger(getClass().getName()).log(System.Logger.Level.TRACE,
                         "Ignored exception during statement clean up", e);

src/main/org/firebirdsql/gds/ng/wire/version11/V11Statement.java

@@ -269,7 +269,9 @@ public WarningMessageCallback getWarningMessageCallback() {
 
                     @Override
                     public boolean requiresSync() {
-                        return option == ISCConstants.DSQL_close;
+                        // DSQL_close requires sync as it isn't flushed,
+                        // other options require sync because server defers response
+                        return true;
                     }
                 });
             } catch (IOException e) {