Skip to content

Commit 0b672e8

Browse files
authored
Fix handling of sqlite transactions (#136)
* Move as much work as possible out of the sqlite txn In particular, don't do anything that could throw in the finally block, as this will mask any exception thrown within the transaction itself. * Use sqlite transaction listeners (rather than doing so manually) Using sqlite's version allows us to not worry about getting the transaction semantics correct in the face of exceptions.
1 parent c9f213c commit 0b672e8

File tree

1 file changed

+23
-10
lines changed

1 file changed

+23
-10
lines changed

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLitePersistence.java

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import android.database.sqlite.SQLiteOpenHelper;
2727
import android.database.sqlite.SQLiteProgram;
2828
import android.database.sqlite.SQLiteStatement;
29+
import android.database.sqlite.SQLiteTransactionListener;
2930
import android.support.annotation.VisibleForTesting;
3031
import com.google.common.base.Function;
3132
import com.google.firebase.firestore.auth.User;
@@ -75,6 +76,21 @@ public static String databaseName(String persistenceKey, DatabaseId databaseId)
7576
private final SQLiteQueryCache queryCache;
7677
private final SQLiteRemoteDocumentCache remoteDocumentCache;
7778
private final SQLiteLruReferenceDelegate referenceDelegate;
79+
private final SQLiteTransactionListener transactionListener =
80+
new SQLiteTransactionListener() {
81+
@Override
82+
public void onBegin() {
83+
referenceDelegate.onTransactionStarted();
84+
}
85+
86+
@Override
87+
public void onCommit() {
88+
referenceDelegate.onTransactionCommitted();
89+
}
90+
91+
@Override
92+
public void onRollback() {}
93+
};
7894

7995
public SQLitePersistence(
8096
Context context, String persistenceKey, DatabaseId databaseId, LocalSerializer serializer) {
@@ -143,35 +159,32 @@ RemoteDocumentCache getRemoteDocumentCache() {
143159

144160
@Override
145161
void runTransaction(String action, Runnable operation) {
162+
Logger.debug(TAG, "Starting transaction: %s", action);
163+
db.beginTransactionWithListener(transactionListener);
146164
try {
147-
Logger.debug(TAG, "Starting transaction: %s", action);
148-
referenceDelegate.onTransactionStarted();
149-
db.beginTransaction();
150165
operation.run();
151166

152167
// Note that an exception in operation.run() will prevent this code from running.
153168
db.setTransactionSuccessful();
154169
} finally {
155170
db.endTransaction();
156-
referenceDelegate.onTransactionCommitted();
157171
}
158172
}
159173

160174
@Override
161175
<T> T runTransaction(String action, Supplier<T> operation) {
176+
Logger.debug(TAG, "Starting transaction: %s", action);
177+
T value = null;
178+
db.beginTransactionWithListener(transactionListener);
162179
try {
163-
Logger.debug(TAG, "Starting transaction: %s", action);
164-
referenceDelegate.onTransactionStarted();
165-
db.beginTransaction();
166-
T value = operation.get();
180+
value = operation.get();
167181

168182
// Note that an exception in operation.run() will prevent this code from running.
169183
db.setTransactionSuccessful();
170-
return value;
171184
} finally {
172185
db.endTransaction();
173-
referenceDelegate.onTransactionCommitted();
174186
}
187+
return value;
175188
}
176189

177190
/**

0 commit comments

Comments
 (0)