Skip to content

Commit fd85d24

Browse files
mrotteveelmrotteveel
committed
#953 Make transaction completion more robust
1 parent a55434f commit fd85d24

12 files changed

Lines changed: 112 additions & 42 deletions

File tree

‎jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaDatabase.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,7 @@ protected void setDetachedJna() {
6868

6969
@Override
7070
protected final void checkConnected() throws SQLException {
71-
if (!isConnected()) {
72-
throw FbExceptionBuilder.toException(JaybirdErrorCodes.jb_notAttachedToDatabase);
73-
}
71+
checkAttached();
7472
}
7573

7674
@Override

‎jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaService.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import com.sun.jna.ptr.IntByReference;
66
import org.firebirdsql.gds.ISCConstants;
7-
import org.firebirdsql.gds.JaybirdErrorCodes;
87
import org.firebirdsql.gds.ServiceParameterBuffer;
98
import org.firebirdsql.gds.ServiceRequestBuffer;
109
import org.firebirdsql.gds.impl.ServiceParameterBufferImp;
@@ -68,9 +67,7 @@ public ServiceRequestBuffer createServiceRequestBuffer() {
6867

6968
@Override
7069
protected void checkConnected() throws SQLException {
71-
if (!isAttached()) {
72-
throw FbExceptionBuilder.toException(JaybirdErrorCodes.jb_notAttachedToDatabase);
73-
}
70+
checkAttached();
7471
}
7572

7673
@Override

‎jaybird-native/src/main/java/org/firebirdsql/gds/ng/jna/JnaTransaction.java‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.firebirdsql.gds.ng.TransactionState;
1111
import org.firebirdsql.gds.ng.listeners.DatabaseListener;
1212
import org.firebirdsql.jaybird.util.Cleaners;
13+
import org.firebirdsql.jaybird.xca.FatalErrorHelper;
1314
import org.firebirdsql.jna.fbclient.FbClientLibrary;
1415
import org.firebirdsql.jna.fbclient.ISC_STATUS;
1516
import org.jspecify.annotations.Nullable;
@@ -68,14 +69,16 @@ public IntByReference getJnaHandle() {
6869
@Override
6970
public void commit() throws SQLException {
7071
try (LockCloseable ignored = withLock()) {
71-
final JnaDatabase db = getDatabase();
72-
db.checkConnected();
72+
checkDbAttached();
7373
switchState(TransactionState.COMMITTING);
7474
clientLibrary.isc_commit_transaction(statusVector, handle);
7575
processStatusVector();
7676
switchState(TransactionState.COMMITTED);
77-
cleanable.clean();
77+
clean();
7878
} catch (SQLException e) {
79+
if (FatalErrorHelper.isBrokenConnection(e)) {
80+
forceAbortedUnknownState();
81+
}
7982
exceptionListenerDispatcher.errorOccurred(e);
8083
throw e;
8184
} finally {
@@ -86,14 +89,16 @@ public void commit() throws SQLException {
8689
@Override
8790
public void rollback() throws SQLException {
8891
try (LockCloseable ignored = withLock()) {
89-
final JnaDatabase db = getDatabase();
90-
db.checkConnected();
92+
checkDbAttached();
9193
switchState(TransactionState.ROLLING_BACK);
9294
clientLibrary.isc_rollback_transaction(statusVector, handle);
9395
processStatusVector();
9496
switchState(TransactionState.ROLLED_BACK);
95-
cleanable.clean();
97+
clean();
9698
} catch (SQLException e) {
99+
if (FatalErrorHelper.isBrokenConnection(e)) {
100+
forceAbortedUnknownState();
101+
}
97102
exceptionListenerDispatcher.errorOccurred(e);
98103
throw e;
99104
} finally {
@@ -105,8 +110,7 @@ public void rollback() throws SQLException {
105110
public void prepare(byte @Nullable [] recoveryInformation) throws SQLException {
106111
boolean noRecoveryInfo = recoveryInformation == null || recoveryInformation.length == 0;
107112
try (LockCloseable ignored = withLock()) {
108-
final JnaDatabase db = getDatabase();
109-
db.checkConnected();
113+
checkDbAttached();
110114
switchState(TransactionState.PREPARING);
111115
if (noRecoveryInfo) {
112116
clientLibrary.isc_prepare_transaction(statusVector, handle);
@@ -117,6 +121,9 @@ public void prepare(byte @Nullable [] recoveryInformation) throws SQLException {
117121
processStatusVector();
118122
switchState(TransactionState.PREPARED);
119123
} catch (SQLException e) {
124+
if (FatalErrorHelper.isBrokenConnection(e)) {
125+
forceAbortedUnknownState();
126+
}
120127
exceptionListenerDispatcher.errorOccurred(e);
121128
throw e;
122129
} finally {
@@ -129,8 +136,7 @@ public byte[] getTransactionInfo(byte[] requestItems, int maxBufferLength) throw
129136
try {
130137
final ByteBuffer responseBuffer = ByteBuffer.allocateDirect(maxBufferLength);
131138
try (LockCloseable ignored = withLock()) {
132-
final JnaDatabase db = getDatabase();
133-
db.checkConnected();
139+
checkDbAttached();
134140
clientLibrary.isc_transaction_info(statusVector, handle, (short) requestItems.length, requestItems,
135141
(short) maxBufferLength, responseBuffer);
136142
processStatusVector();
@@ -148,6 +154,16 @@ private void processStatusVector() throws SQLException {
148154
getDatabase().processStatusVector(statusVector, null);
149155
}
150156

157+
@Override
158+
protected void forceAbortedUnknownState() {
159+
super.forceAbortedUnknownState();
160+
clean();
161+
}
162+
163+
private void clean() {
164+
cleanable.clean();
165+
}
166+
151167
private static final class CleanupAction implements Runnable, DatabaseListener {
152168

153169
private final IntByReference handle;

‎src/main/org/firebirdsql/gds/ng/AbstractFbAttachment.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ public final void removeExceptionListener(ExceptionListener listener) {
149149

150150
/**
151151
* Checks if the attachment is connected, and throws a {@link SQLException} if it isn't connected.
152+
* <p>
153+
* Implementations where connected and attached are indistinguishable may call {@link #checkAttached()} or
154+
* vice versa.
155+
* </p>
152156
*/
153157
protected abstract void checkConnected() throws SQLException;
154158

‎src/main/org/firebirdsql/gds/ng/AbstractFbTransaction.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,15 @@ protected final void switchState(final TransactionState newState) throws SQLExce
7878
}
7979
}
8080

81+
protected void forceAbortedUnknownState() {
82+
try (var ignored = withLock()) {
83+
final TransactionState currentState = state;
84+
if (currentState == TransactionState.ABORTED_UNKNOWN) return;
85+
state = TransactionState.ABORTED_UNKNOWN;
86+
transactionListenerDispatcher.transactionStateChanged(this, TransactionState.ABORTED_UNKNOWN, currentState);
87+
}
88+
}
89+
8190
@Override
8291
public final void addTransactionListener(TransactionListener listener) {
8392
transactionListenerDispatcher.addListener(listener);
@@ -141,6 +150,10 @@ protected FbDatabase getDatabase() {
141150
return database;
142151
}
143152

153+
protected void checkDbAttached() throws SQLException {
154+
database.checkAttached();
155+
}
156+
144157
protected final void logUnexpectedState(TransactionState expectedState, System.Logger log) {
145158
final TransactionState transactionState = getState();
146159
if (transactionState != expectedState && log.isLoggable(System.Logger.Level.WARNING)) {

‎src/main/org/firebirdsql/gds/ng/FbAttachment.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
// SPDX-FileCopyrightText: Copyright 2013-2025 Mark Rotteveel
1+
// SPDX-FileCopyrightText: Copyright 2013-2026 Mark Rotteveel
22
// SPDX-License-Identifier: LGPL-2.1-or-later OR BSD-3-Clause
33
package org.firebirdsql.gds.ng;
44

55
import org.firebirdsql.encodings.Encoding;
66
import org.firebirdsql.encodings.IEncodingFactory;
7+
import org.firebirdsql.gds.JaybirdErrorCodes;
78
import org.firebirdsql.gds.impl.GDSServerVersion;
89
import org.firebirdsql.gds.ng.listeners.ExceptionListenable;
910

@@ -62,10 +63,26 @@ public interface FbAttachment extends AutoCloseable, ExceptionListenable {
6263
/**
6364
* Current attachment status.
6465
*
65-
* @return {@code true} if connected to the server and attached to a database or service, {@code false} otherwise.
66+
* @return {@code true} if connected to the server and attached to a database or service, {@code false} otherwise
67+
* @see #checkAttached()
6668
*/
6769
boolean isAttached();
6870

71+
/**
72+
* Checks if a physical connection to the server is established and if the connection is attached to a database or
73+
* service.
74+
*
75+
* @throws SQLException
76+
* if the database or service is not connected or attached
77+
* @see #isAttached()
78+
* @since 7
79+
*/
80+
default void checkAttached() throws SQLException {
81+
if (!isAttached()) {
82+
throw FbExceptionBuilder.forException(JaybirdErrorCodes.jb_notAttachedToDatabase).toSQLException();
83+
}
84+
}
85+
6986
/**
7087
* @return The {@link IEncodingFactory} for this connection
7188
*/

‎src/main/org/firebirdsql/gds/ng/TransactionState.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,26 @@ Set<TransactionState> createValidTransitionSet() {
5757
Set<TransactionState> createValidTransitionSet() {
5858
return EnumSet.noneOf(TransactionState.class);
5959
}
60+
},
61+
/**
62+
* State used when a transaction was aborted with an unknown result. For example, failure to commit, rollback, or
63+
* prepare due to broken connections (IO errors, or the connection already being closed).
64+
* <p>
65+
* In most cases, the transaction is probably rolled back, but we're not sure. For example, the commit could have
66+
* been received and processed by the server, but a subsequent connection failure occurred before the response was
67+
* received. Similar for a transaction prepare (first step of two-phase commit).
68+
* </p>
69+
* <p>
70+
* There are no valid transitions to and from this state; it can only be forcibly set.
71+
* </p>
72+
*
73+
* @since 7
74+
*/
75+
ABORTED_UNKNOWN {
76+
@Override
77+
Set<TransactionState> createValidTransitionSet() {
78+
return EnumSet.noneOf(TransactionState.class);
79+
}
6080
};
6181

6282
private @Nullable Set<TransactionState> validTransitions;

‎src/main/org/firebirdsql/gds/ng/wire/AbstractFbWireDatabase.java‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import java.sql.SQLException;
1313

1414
import static java.util.Objects.requireNonNull;
15-
import static org.firebirdsql.gds.JaybirdErrorCodes.jb_notAttachedToDatabase;
1615
import static org.firebirdsql.gds.JaybirdErrorCodes.jb_notConnectedToServer;
1716
import static org.firebirdsql.gds.JaybirdErrorCodes.jb_unableToCancelEventReasonNotConnected;
1817
import static org.firebirdsql.gds.impl.wire.WireProtocolConstants.PROTOCOL_VERSION18;
@@ -126,21 +125,17 @@ protected final boolean isConnected() {
126125
}
127126

128127
/**
129-
* Checks if a physical connection to the server is established and if the
130-
* connection is attached to a database.
128+
* Checks if a physical connection to the server is established and if the connection is attached to a database.
131129
* <p>
132-
* This method calls {@link #checkConnected()}, so it is not necessary to
133-
* call both.
130+
* This method calls {@link #checkConnected()}, so it is not necessary to call both.
134131
* </p>
135132
*
136133
* @throws SQLException
137-
* If the database not connected or attached.
134+
* if the database is not connected or attached
138135
*/
139-
protected final void checkAttached() throws SQLException {
136+
public final void checkAttached() throws SQLException {
140137
checkConnected();
141-
if (!isAttached()) {
142-
throw FbExceptionBuilder.toNonTransientConnectionException(jb_notAttachedToDatabase);
143-
}
138+
super.checkAttached();
144139
}
145140

146141
/**

‎src/main/org/firebirdsql/gds/ng/wire/AbstractFbWireService.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,17 @@ protected final boolean isConnected() {
104104
}
105105

106106
/**
107-
* Checks if a physical connection to the server is established and if the
108-
* connection is attached to a database.
107+
* Checks if a physical connection to the server is established and if the connection is attached to a database.
109108
* <p>
110-
* This method calls {@link #checkConnected()}, so it is not necessary to
111-
* call both.
109+
* This method calls {@link #checkConnected()}, so it is not necessary to call both.
112110
* </p>
113111
*
114112
* @throws SQLException
115-
* If the database not connected or attached.
113+
* if the service is not connected or attached
116114
*/
117-
protected final void checkAttached() throws SQLException {
115+
public final void checkAttached() throws SQLException {
118116
checkConnected();
119-
if (!isAttached()) {
120-
throw FbExceptionBuilder.toNonTransientConnectionException(JaybirdErrorCodes.jb_notAttachedToDatabase);
121-
}
117+
super.checkAttached();
122118
}
123119

124120
@Override

‎src/main/org/firebirdsql/gds/ng/wire/InlineBlobCache.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ public boolean add(FbTransaction transaction, InlineBlob blob) {
168168
* </p>
169169
*/
170170
private static final Set<TransactionState> CLEAN_CACHE_ON_TRANSACTION_STATES = unmodifiableSet(
171-
EnumSet.of(TransactionState.PREPARED, TransactionState.COMMITTED, TransactionState.ROLLED_BACK));
171+
EnumSet.of(TransactionState.PREPARED, TransactionState.COMMITTED, TransactionState.ROLLED_BACK,
172+
TransactionState.ABORTED_UNKNOWN));
172173

173174
@Override
174175
public void transactionStateChanged(FbTransaction transaction, TransactionState newState,

0 commit comments

Comments
 (0)