#904 Fix Statement.cancel() causes lockup

(partial backport of #903 from Jaybird 7)

Author: mrotteveel

src/docs/asciidoc/release_notes.adoc

@@ -38,6 +38,10 @@ The following has been changed or fixed since Jaybird 5.0.10:
 * Fixed: `FBServiceManager.getAuthPlugins()` reported the `dbCryptConfig` value (https://github.com/FirebirdSQL/jaybird/issues/902[#902])
 +
 This fix was backported from Jaybird 6.0.4.
+* Fixed: `Statement.cancel()` causes lockup (https://github.com/FirebirdSQL/jaybird/issues/904[#904])
++
+This is a partial backport from Jaybird 7.
+See also <<breaking-changes-internal-api-5-0-11>>.
 
 [#jaybird-5-0-10-changelog]
 === Jaybird 5.0.10
@@ -1484,6 +1488,22 @@ This should only affect maintainers of Jaybird forks, or people using the intern
 ** Constructors now throw `SQLException` if the provided database is null or not attached, or the provided transaction is null or not active.
 Previously, passing in a null transaction would result in a `NullPointerException`.
 
+[#breaking-changes-internal-api-5-0-11]
+==== Breaking changes internal API for 5.0.11
+
+With the introduction of the (partial) transmit lock in the wire protocol to address https://github.com/FirebirdSQL/jaybird/issues/892[#892 Statement.cancel() causes lockup], statement and transaction operations must now hold the transmit lock while sending messages to the server.
+See also https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2026-02-cancellation-thread-safety-backport.adoc[jdp-2026-02: Cancellation thread-safety backport].
+
+Implementations and forks *must* ensure they hold the transmit lock _within_ the connection lock while sending statement or transaction related messages.
+The transmit lock *must* be released before receiving (reading) responses from the server.
+
+We have avoided most of the breaking changes that we made in Jaybird 7, but the following is a minor incompatible change:
+
+* V10Statement
+** `sendExecute(int, RowValue)` should no longer be overridden, instead override `sendExecuteMsg(XdrOutputStream, int, RowValue)`.
++
+If you do continue to override `sendExecute(int, RowValue)`, make sure the transmit lock covers the entire message.
+
 [#breaking-changes-unlikely]
 === Unlikely breaking changes
 

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

@@ -82,12 +82,16 @@ protected final XdrOutputStream getXdrOut() throws SQLException {
         return getXdrStreamAccess().getXdrOut();
     }
 
-    private XdrStreamAccess getXdrStreamAccess() throws SQLException {
-        if (database != null) {
-            return database.getXdrStreamAccess();
-        } else {
-            throw new SQLException("Connection closed or no connection available");
-        }
+    /**
+     * @see XdrStreamAccess#withTransmitLock(TransmitAction)
+     * @since 5.0.11
+     */
+    protected final void withTransmitLock(TransmitAction transmitAction) throws IOException, SQLException {
+        getXdrStreamAccess().withTransmitLock(transmitAction);
+    }
+
+    private XdrStreamAccess getXdrStreamAccess() {
+        return database.getXdrStreamAccess();
     }
 
     @Override

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

@@ -0,0 +1,63 @@
+/*
+ * Public Firebird Java API.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *    1. Redistributions of source code must retain the above copyright notice,
+ *       this list of conditions and the following disclaimer.
+ *    2. Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *    3. The name of the author may not be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+ * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
+ * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
+ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.firebirdsql.gds.ng.wire;
+
+import org.firebirdsql.gds.impl.wire.XdrOutputStream;
+
+import java.io.IOException;
+import java.sql.SQLException;
+
+/**
+ * Action for transmitting messages to Firebird.
+ * <p>
+ * This action is primarily intended for transmitting data while the transmit lock is held.
+ * </p>
+ *
+ * @see XdrStreamAccess#withTransmitLock(TransmitAction)
+ * @since 5.0.11
+ */
+@FunctionalInterface
+public interface TransmitAction {
+
+    /**
+     * Transmits a message (or messages) to Firebird.
+     * <p>
+     * Implementations <strong>should not</strong> obtain (additional) locks, and <strong>must not</strong> attempt to
+     * read (receive) data from the server. Preferably, the only operations done are writes to {@code xdrOut}. In
+     * general, an action should write the whole message. If that is not possible, make sure that the full message is
+     * written while the transmit lock is held.
+     * </p>
+     *
+     * @param xdrOut
+     *         XDR output stream to Firebird server
+     * @throws IOException
+     *         for errors writing into {@code xdrOut}
+     * @throws SQLException
+     *         for connection state errors
+     * @see XdrStreamAccess#withTransmitLock(TransmitAction)
+     */
+    void transmit(XdrOutputStream xdrOut) throws IOException, SQLException;
+
+}

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

@@ -57,6 +57,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
 
 import static org.firebirdsql.gds.impl.wire.WireProtocolConstants.*;
 
@@ -91,6 +92,9 @@ public abstract class WireConnection<T extends IAttachProperties<T>, C extends F
     private XdrOutputStream xdrOut;
     private XdrInputStream xdrIn;
     private final XdrStreamAccess streamAccess = new XdrStreamAccess() {
+
+        private final ReentrantLock transmitLock = new ReentrantLock();
+
         @Override
         public XdrInputStream getXdrIn() throws SQLException {
             if (isConnected() && xdrIn != null) {
@@ -108,6 +112,17 @@ public XdrOutputStream getXdrOut() throws SQLException {
                 throw new SQLException("Connection closed or no connection available");
             }
         }
+
+        @Override
+        public void withTransmitLock(TransmitAction transmitAction) throws IOException, SQLException {
+            transmitLock.lock();
+            try {
+                transmitAction.transmit(getXdrOut());
+            } finally {
+                transmitLock.unlock();
+            }
+        }
+
     };
 
     /**
@@ -599,7 +614,18 @@ private static String getSystemPropertyPrivileged(final String propertyName) {
      *         If there is no socket, the socket is closed, or for errors writing to the socket.
      */
     public final void writeDirect(byte[] data) throws IOException {
-        xdrOut.writeDirect(data);
+        try {
+            // NOTE: In Jaybird 5 and 6, this is not fully thread-safe, as the transmit lock only covers cancellation
+            // and statement operations; a full solution is available in Jaybird 7 and higher
+            getXdrStreamAccess().withTransmitLock(xdrOut -> {
+                xdrOut.flush();
+                xdrOut.writeDirect(data);
+            });
+        } catch (SQLException e) {
+            // This can happen when withTransmitLock is called on a broken connection; we don't want to change
+            // the signature of this method, so we convert to an IOException
+            throw new IOException(e);
+        }
     }
 
     final List<KnownServerKey.PluginSpecificData> getPluginSpecificData() {

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

@@ -1,6 +1,4 @@
 /*
- * $Id$
- *
  * Public Firebird Java API.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -29,6 +27,7 @@
 import org.firebirdsql.gds.impl.wire.XdrInputStream;
 import org.firebirdsql.gds.impl.wire.XdrOutputStream;
 
+import java.io.IOException;
 import java.sql.SQLException;
 
 /**
@@ -40,22 +39,54 @@
 public interface XdrStreamAccess {
 
     /**
-     * Gets the XdrInputStream.
+     * Gets the XDR input stream.
      *
-     * @return Instance of XdrInputStream
+     * @return instance of {@link XdrInputStream}
      * @throws SQLException
-     *         If no connection is opened or when exceptions occur
-     *         retrieving the InputStream
+     *         if no connection is opened or when exceptions occur retrieving the InputStream
      */
     XdrInputStream getXdrIn() throws SQLException;
 
     /**
-     * Gets the XdrOutputStream.
+     * Gets the XDR output stream.
      *
-     * @return Instance of XdrOutputStream
+     * @return instance of {@link XdrOutputStream}
      * @throws SQLException
-     *         If no connection is opened or when exceptions occur
-     *         retrieving the OutputStream
+     *         if no connection is opened or when exceptions occur retrieving the OutputStream
+     * @see #withTransmitLock(TransmitAction)
      */
     XdrOutputStream getXdrOut() throws SQLException;
+
+    /**
+     * Runs {@link TransmitAction#transmit(XdrOutputStream)} with {@link #getXdrOut()} on {@code transmitAction} under
+     * the transmit lock.
+     * <p>
+     * For Jaybird 5 and 6, the transmit lock only needs to be used for statement operations and cancellation. See also
+     * <a href="https://github.com/FirebirdSQL/jaybird/blob/master/devdoc/jdp/jdp-2026-02-cancellation-thread-safety-backport.adoc">jdp-2026-02: Cancellation thread-safety backport</a>
+     * </p>
+     * <p>
+     * The transmit lock should only cover sending messages to the server. It should be held for the duration of the
+     * entire message. It <strong>must</strong> be released <em>before</em> reading (receiving) messages from the
+     * server. If possible, do not do anything other than writing to the XDR output stream while holding the lock.
+     * </p>
+     * <p>
+     * Normal operations <strong>must</strong> obtain the lock while holding the connection lock (i.e. the various
+     * {@code withLock()} methods). Out-of-band operations (e.g. cancellation) <strong>must not</strong> take out the
+     * connection lock, otherwise they can't be out-of-band.
+     * </p>
+     * <p>
+     * Note for implementations: the lock used must be reentrant.
+     * </p>
+     *
+     * @param transmitAction
+     *         the transmit action to run under lock
+     * @throws IOException
+     *         for errors writing to the XDR output stream
+     * @throws SQLException
+     *         for other database access errors
+     * @see TransmitAction
+     * @since 5.0.11
+     */
+    void withTransmitLock(TransmitAction transmitAction) throws IOException, SQLException;
+
 }