Close SQL statement, preventing finalizer crashes

This will stop instances of the following from occuring in the logs
on SMS migration:

W/SQLiteCompiledSql: Releasing statement in a finalizer. Please ensure
that you explicitly call close() on your cursor: INSERT INTO sms
(address, person, date_sent, date, protocol, read, status, type,
reply_path_present,
    net.sqlcipher.database.DatabaseObjectNotClosedException: Application did not close the cursor or database object that was opened here
        at net.sqlcipher.database.SQLiteCompiledSql.<init>(SQLiteCompiledSql.java:62)
        at net.sqlcipher.database.SQLiteProgram.<init>(SQLiteProgram.java:109)
        at net.sqlcipher.database.SQLiteStatement.<init>(SQLiteStatement.java:39)
        at net.sqlcipher.database.SQLiteDatabase.compileStatement(SQLiteDatabase.java:1647)
        at org.thoughtcrime.securesms.database.SmsDatabase.createInsertStatement(SmsDatabase.java:767)
        at org.thoughtcrime.securesms.database.SmsMigrator.migrateConversation(SmsMigrator.java:166)
        at org.thoughtcrime.securesms.database.SmsMigrator.migrateDatabase(SmsMigrator.java:210)
        at org.thoughtcrime.securesms.service.ApplicationMigrationService$ImportRunnable.run(ApplicationMigrationService.java:159)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
        at java.lang.Thread.run(Thread.java:764)

We aren't closing Statement objects before the finalizer on those
objects runs. When the GC runs, we'll get warnings like the above
which alert us to the fact that these objects are being automatically
closed for us in the finalizer, but that this is suboptimal behavior.

If we leave too many Statement (or Cursor) objects to be closed in
their finalizers, when the GC runs, it'll take longer than 10 seconds
to close them all and Android will kill the app. This 10 second limit
is hardcoded and we can only try to avoid it. A crash will look like:

java.util.concurrent.TimeoutException: net.sqlcipher.database.SQLiteCompiledSql.finalize() timed out after 10 seconds
    at java.lang.Object.wait(Native Method)
    at java.lang.Thread.parkFor$(Thread.java:1220)
    at sun.misc.Unsafe.park(Unsafe.java:299)
    at java.util.concurrent.locks.LockSupport.park(LockSupport.java:158)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:810)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireQueued(AbstractQueuedSynchronizer.java:844)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:1173)
    at java.util.concurrent.locks.ReentrantLock$FairSync.lock(ReentrantLock.java:196)
    at java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:257)
    at net.sqlcipher.database.SQLiteDatabase.lock(SQLiteDatabase.java:553)
    at net.sqlcipher.database.SQLiteCompiledSql.releaseSqlStatement(SQLiteCompiledSql.java:106)
    at net.sqlcipher.database.SQLiteCompiledSql.finalize(SQLiteCompiledSql.java:152)
    at java.lang.Daemons$FinalizerDaemon.doFinalize(Daemons.java:202)
    at java.lang.Daemons$FinalizerDaemon.run(Daemons.java:185)
    at java.lang.Thread.run(Thread.java:818)

I was able to replicate the above crash consistently on a
Samsung Galaxy S7 edge when importing well over 100k SMS messages.
But as soon as I attached a debugger the crash did not persist. I
assume this is because of some VM-level interactions between the two
and did not investigate further after fixing it.

I do not have access to the stack trace for issue #7953 but this
could potentially resolve it. The crash is identical to that in #7477
but this patch is for SMS migration not restoring from a backup. I
was not able to replicate the crash on restoring a >100k message
backup.
This commit is contained in:
Kevin Mark 2018-10-07 07:11:15 -04:00 committed by Greyson Parrelli
parent 8ad5126408
commit 88f9ec313f

View File

@ -150,6 +150,7 @@ public class SmsMigrator {
{ {
SmsDatabase ourSmsDatabase = DatabaseFactory.getSmsDatabase(context); SmsDatabase ourSmsDatabase = DatabaseFactory.getSmsDatabase(context);
Cursor cursor = null; Cursor cursor = null;
SQLiteStatement statement = null;
try { try {
Uri uri = Uri.parse("content://sms/conversations/" + theirThreadId); Uri uri = Uri.parse("content://sms/conversations/" + theirThreadId);
@ -163,7 +164,7 @@ public class SmsMigrator {
} }
SQLiteDatabase transaction = ourSmsDatabase.beginTransaction(); SQLiteDatabase transaction = ourSmsDatabase.beginTransaction();
SQLiteStatement statement = ourSmsDatabase.createInsertStatement(transaction); statement = ourSmsDatabase.createInsertStatement(transaction);
while (cursor != null && cursor.moveToNext()) { while (cursor != null && cursor.moveToNext()) {
int typeColumn = cursor.getColumnIndex(SmsDatabase.TYPE); int typeColumn = cursor.getColumnIndex(SmsDatabase.TYPE);
@ -181,6 +182,8 @@ public class SmsMigrator {
DatabaseFactory.getThreadDatabase(context).notifyConversationListeners(ourThreadId); DatabaseFactory.getThreadDatabase(context).notifyConversationListeners(ourThreadId);
} finally { } finally {
if (statement != null)
statement.close();
if (cursor != null) if (cursor != null)
cursor.close(); cursor.close();
} }