Skip to content

Commit 7a3f72f

Browse files
mrotteveelmrotteveel
committed
#953 Make transaction completion more robust
1 parent 87696ec commit 7a3f72f

13 files changed

Lines changed: 116 additions & 39 deletions

File tree

‎src/docs/asciidoc/release_notes.adoc‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ This change was backported from Jaybird 6.0.6.
5959
Fix was contributed by Artyom Abakumov, and backport from Jaybird 6.0.6.
6060
* Improvement: Support customization of service requests in service managers (issue:[947])
6161
* Dependency update: updated `net.java.dev.jna:jna` from 5.18.1 to 5.19.1 (used by `jaybird-native`) (issue:[952])
62+
* Added `TransactionState.ABORTED_UNKNOWN`.
63+
+
64+
This state is used when a commit, rollback, or prepare failed with an exception that Jaybird considers to indicate a broken connection.
65+
For end-users of Jaybird, this change has no real impact, except it may change the state logged for certain failures ending a transaction.
6266
6367
[#jaybird-5-0-12-changelog]
6468
=== Jaybird 5.0.12
@@ -1756,6 +1760,17 @@ Other breaking changes to the internal API in Jaybird 5.0.11 unrelated to the tr
17561760
* `FbWireOperations`
17571761
** `readNextOperation` now also throws `SQLException` -- if the connection is closed.
17581762

1763+
[#breaking-changes-internal-api-5-0-13]
1764+
==== Breaking changes internal API for 5.0.13
1765+
1766+
A new terminal transaction state `ABORTED_UNKNOWN` was added.
1767+
This state is used when commit, rollback, or prepare fail with an exception indicating a broken connection.
1768+
1769+
It can be reported for state changes from `ACTIVE`, `COMMITTING`, `ROLLING_BACK`, `PREPARING` and `PREPARED`.
1770+
1771+
The interface `FbAttachment` received a method `checkAttached() throws SQLException` (with a default implementation).
1772+
Protected methods of the same name in subclasses are now public.
1773+
17591774
[#breaking-changes-unlikely]
17601775
=== Unlikely breaking changes
17611776

‎src/jna-client/org/firebirdsql/gds/ng/jna/JnaDatabase.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,7 @@ protected final FbClientLibrary getClientLibrary() {
8181

8282
@Override
8383
protected void checkConnected() throws SQLException {
84-
if (!isAttached()) {
85-
throw FbExceptionBuilder.forException(JaybirdErrorCodes.jb_notAttachedToDatabase).toSQLException();
86-
}
84+
checkAttached();
8785
}
8886

8987
@Override

‎src/jna-client/org/firebirdsql/gds/ng/jna/JnaService.java‎

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

2121
import com.sun.jna.ptr.IntByReference;
2222
import org.firebirdsql.gds.ISCConstants;
23-
import org.firebirdsql.gds.JaybirdErrorCodes;
2423
import org.firebirdsql.gds.ServiceParameterBuffer;
2524
import org.firebirdsql.gds.ServiceRequestBuffer;
2625
import org.firebirdsql.gds.impl.ServiceParameterBufferImp;
@@ -73,9 +72,7 @@ public ServiceRequestBuffer createServiceRequestBuffer() {
7372

7473
@Override
7574
protected void checkConnected() throws SQLException {
76-
if (!isAttached()) {
77-
throw FbExceptionBuilder.forException(JaybirdErrorCodes.jb_notAttachedToDatabase).toSQLException();
78-
}
75+
checkAttached();
7976
}
8077

8178
@Override

‎src/jna-client/org/firebirdsql/gds/ng/jna/JnaTransaction.java‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.firebirdsql.gds.ng.AbstractFbTransaction;
2323
import org.firebirdsql.gds.ng.LockCloseable;
2424
import org.firebirdsql.gds.ng.TransactionState;
25+
import org.firebirdsql.jaybird.xca.FatalErrorHelper;
2526
import org.firebirdsql.jna.fbclient.FbClientLibrary;
2627
import org.firebirdsql.jna.fbclient.ISC_STATUS;
2728
import org.firebirdsql.logging.Logger;
@@ -78,13 +79,15 @@ public IntByReference getJnaHandle() {
7879
@Override
7980
public void commit() throws SQLException {
8081
try (LockCloseable ignored = withLock()) {
81-
final JnaDatabase db = getDatabase();
82-
db.checkConnected();
82+
checkDbAttached();
8383
switchState(TransactionState.COMMITTING);
8484
clientLibrary.isc_commit_transaction(statusVector, handle);
8585
processStatusVector();
8686
switchState(TransactionState.COMMITTED);
8787
} catch (SQLException e) {
88+
if (FatalErrorHelper.isBrokenConnection(e)) {
89+
forceAbortedUnknownState();
90+
}
8891
exceptionListenerDispatcher.errorOccurred(e);
8992
throw e;
9093
} finally {
@@ -103,13 +106,15 @@ public void commit() throws SQLException {
103106
@Override
104107
public void rollback() throws SQLException {
105108
try (LockCloseable ignored = withLock()) {
106-
final JnaDatabase db = getDatabase();
107-
db.checkConnected();
109+
checkDbAttached();
108110
switchState(TransactionState.ROLLING_BACK);
109111
clientLibrary.isc_rollback_transaction(statusVector, handle);
110112
processStatusVector();
111113
switchState(TransactionState.ROLLED_BACK);
112114
} catch (SQLException e) {
115+
if (FatalErrorHelper.isBrokenConnection(e)) {
116+
forceAbortedUnknownState();
117+
}
113118
exceptionListenerDispatcher.errorOccurred(e);
114119
throw e;
115120
} finally {
@@ -129,8 +134,7 @@ public void rollback() throws SQLException {
129134
public void prepare(byte[] recoveryInformation) throws SQLException {
130135
boolean noRecoveryInfo = recoveryInformation == null || recoveryInformation.length == 0;
131136
try (LockCloseable ignored = withLock()) {
132-
final JnaDatabase db = getDatabase();
133-
db.checkConnected();
137+
checkDbAttached();
134138
switchState(TransactionState.PREPARING);
135139
if (noRecoveryInfo) {
136140
clientLibrary.isc_prepare_transaction(statusVector, handle);
@@ -141,6 +145,9 @@ public void prepare(byte[] recoveryInformation) throws SQLException {
141145
processStatusVector();
142146
switchState(TransactionState.PREPARED);
143147
} catch (SQLException e) {
148+
if (FatalErrorHelper.isBrokenConnection(e)) {
149+
forceAbortedUnknownState();
150+
}
144151
exceptionListenerDispatcher.errorOccurred(e);
145152
throw e;
146153
} finally {
@@ -161,8 +168,7 @@ public byte[] getTransactionInfo(byte[] requestItems, int maxBufferLength) throw
161168
try {
162169
final ByteBuffer responseBuffer = ByteBuffer.allocateDirect(maxBufferLength);
163170
try (LockCloseable ignored = withLock()) {
164-
final JnaDatabase db = getDatabase();
165-
db.checkConnected();
171+
checkDbAttached();
166172
clientLibrary.isc_transaction_info(statusVector, handle, (short) requestItems.length, requestItems,
167173
(short) maxBufferLength, responseBuffer);
168174
processStatusVector();

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

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

166166
/**
167167
* Checks if the attachment is connected, and throws a {@link SQLException} if it isn't connected.
168+
* <p>
169+
* Implementations where connected and attached are indistinguishable may call {@link #checkAttached()} or
170+
* vice versa.
171+
* </p>
168172
*/
169173
protected abstract void checkConnected() throws SQLException;
170174

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ protected final void switchState(final TransactionState newState) throws SQLExce
9393
}
9494
}
9595

96+
protected final void forceAbortedUnknownState() throws SQLException {
97+
try (LockCloseable ignored = withLock()) {
98+
final TransactionState currentState = state;
99+
if (currentState == TransactionState.ABORTED_UNKNOWN) return;
100+
state = TransactionState.ABORTED_UNKNOWN;
101+
transactionListenerDispatcher.transactionStateChanged(this, TransactionState.ABORTED_UNKNOWN, currentState);
102+
}
103+
}
104+
96105
@Override
97106
public final void addTransactionListener(TransactionListener listener) {
98107
transactionListenerDispatcher.addListener(listener);
@@ -169,4 +178,9 @@ protected void finalize() throws Throwable {
169178
protected FbDatabase getDatabase() {
170179
return database;
171180
}
181+
182+
protected void checkDbAttached() throws SQLException {
183+
database.checkAttached();
184+
}
185+
172186
}

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import org.firebirdsql.encodings.Encoding;
2828
import org.firebirdsql.encodings.IEncodingFactory;
29+
import org.firebirdsql.gds.JaybirdErrorCodes;
2930
import org.firebirdsql.gds.impl.GDSServerVersion;
3031
import org.firebirdsql.gds.ng.listeners.ExceptionListenable;
3132

@@ -81,10 +82,26 @@ public interface FbAttachment extends AutoCloseable, ExceptionListenable {
8182
/**
8283
* Current attachment status.
8384
*
84-
* @return {@code true} if connected to the server and attached to a database or service, {@code false} otherwise.
85+
* @return {@code true} if connected to the server and attached to a database or service, {@code false} otherwise
86+
* @see #checkAttached()
8587
*/
8688
boolean isAttached();
8789

90+
/**
91+
* Checks if a physical connection to the server is established and if the connection is attached to a database or
92+
* service.
93+
*
94+
* @throws SQLException
95+
* if the database or service is not connected or attached
96+
* @see #isAttached()
97+
* @since 5.0.13
98+
*/
99+
default void checkAttached() throws SQLException {
100+
if (!isAttached()) {
101+
throw FbExceptionBuilder.forException(JaybirdErrorCodes.jb_notAttachedToDatabase).toSQLException();
102+
}
103+
}
104+
88105
/**
89106
* @return The {@link IEncodingFactory} for this connection
90107
*/

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
/*
2-
* $Id$
3-
*
42
* Public Firebird Java API.
53
*
64
* Redistribution and use in source and binary forms, with or without
@@ -79,6 +77,26 @@ Set<TransactionState> createValidTransitionSet() {
7977
Set<TransactionState> createValidTransitionSet() {
8078
return EnumSet.noneOf(TransactionState.class);
8179
}
80+
},
81+
/**
82+
* State used when a transaction was aborted with an unknown result. For example, failure to commit, rollback, or
83+
* prepare due to broken connections (IO errors, or the connection already being closed).
84+
* <p>
85+
* In most cases, the transaction is probably rolled back, but we're not sure. For example, the commit could have
86+
* been received and processed by the server, but a subsequent connection failure occurred before the response was
87+
* received. Similar for a transaction prepare (first step of two-phase commit).
88+
* </p>
89+
* <p>
90+
* There are no valid transitions to and from this state; it can only be forcibly set.
91+
* </p>
92+
*
93+
* @since 5.0.13
94+
*/
95+
ABORTED_UNKNOWN {
96+
@Override
97+
Set<TransactionState> createValidTransitionSet() {
98+
return EnumSet.noneOf(TransactionState.class);
99+
}
82100
};
83101

84102
private Set<TransactionState> validTransitions;

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,21 +137,18 @@ protected final void checkConnected() throws SQLException {
137137
}
138138

139139
/**
140-
* Checks if a physical connection to the server is established and if the
141-
* connection is attached to a database.
140+
* Checks if a physical connection to the server is established and if the connection is attached to a database.
142141
* <p>
143-
* This method calls {@link #checkConnected()}, so it is not necessary to
144-
* call both.
142+
* This method calls {@link #checkConnected()}, so it is not necessary to call both.
145143
* </p>
146144
*
147145
* @throws SQLException
148-
* If the database not connected or attached.
146+
* if the database is not connected or attached
149147
*/
150-
protected final void checkAttached() throws SQLException {
148+
@Override
149+
public final void checkAttached() throws SQLException {
151150
checkConnected();
152-
if (!isAttached()) {
153-
throw FbExceptionBuilder.forException(JaybirdErrorCodes.jb_notAttachedToDatabase).toSQLException();
154-
}
151+
super.checkAttached();
155152
}
156153

157154
/**

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -111,21 +111,18 @@ protected final void checkConnected() throws SQLException {
111111
}
112112

113113
/**
114-
* Checks if a physical connection to the server is established and if the
115-
* connection is attached to a database.
114+
* Checks if a physical connection to the server is established and if the connection is attached to a database.
116115
* <p>
117-
* This method calls {@link #checkConnected()}, so it is not necessary to
118-
* call both.
116+
* This method calls {@link #checkConnected()}, so it is not necessary to call both.
119117
* </p>
120118
*
121119
* @throws SQLException
122-
* If the database not connected or attached.
120+
* if the service is not connected or attached
123121
*/
124-
protected final void checkAttached() throws SQLException {
122+
@Override
123+
public final void checkAttached() throws SQLException {
125124
checkConnected();
126-
if (!isAttached()) {
127-
throw FbExceptionBuilder.forException(JaybirdErrorCodes.jb_notAttachedToDatabase).toSQLException();
128-
}
125+
super.checkAttached();
129126
}
130127

131128
@Override

0 commit comments

Comments
 (0)