Skip to content

net.sqlcipher.database.SQLiteDatabase.native_execSQL #555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
allenhsu-17live opened this issue Jul 2, 2021 · 7 comments
Closed

net.sqlcipher.database.SQLiteDatabase.native_execSQL #555

allenhsu-17live opened this issue Jul 2, 2021 · 7 comments
Labels
stale This issue lacks recent activity.

Comments

@allenhsu-17live
Copy link

allenhsu-17live commented Jul 2, 2021

Expected Behavior

No crash

Actual Behavior

crash happen from firebase

net.sqlcipher.database.SQLiteDatabase.native_execSQL (SQLiteDatabase.java)
net.sqlcipher.database.SQLiteDatabase.execSQL (SQLiteDatabase.java:2439)
net.sqlcipher.database.SQLiteDatabase.beginTransactionWithListenerInternal (SQLiteDatabase.java:3052)
net.sqlcipher.database.SQLiteDatabase.beginTransactionWithListener (SQLiteDatabase.java:777)
net.sqlcipher.database.SQLiteDatabase.beginTransaction (SQLiteDatabase.java:748)
androidx.room.InvalidationTracker.syncTriggers (InvalidationTracker.java:498)
androidx.room.InvalidationTracker.syncTriggers (InvalidationTracker.java:538)
androidx.room.InvalidationTracker.addObserver (InvalidationTracker.java:275)
androidx.room.MultiInstanceInvalidationClient$3.run (MultiInstanceInvalidationClient.java:124)
java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1167)
java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:641)
java.lang.Thread.run (Thread.java:764)

I guess the rootcase on here

void syncTriggers(SupportSQLiteDatabase database) {
        if (database.inTransaction()) {
            // we won't run this inside another transaction.
            return;
        }
        try {
            // This method runs in a while loop because while changes are synced to db, another
            // runnable may be skipped. If we cause it to skip, we need to do its work.
            while (true) {
                Lock closeLock = mDatabase.getCloseLock();
                closeLock.lock();
                try {
                    // there is a potential race condition where another mSyncTriggers runnable
                    // can start running right after we get the tables list to sync.
                    final int[] tablesToSync = mObservedTableTracker.getTablesToSync();
                    if (tablesToSync == null) {
                        return;
                    }
                    final int limit = tablesToSync.length;
                    database.beginTransaction();
                    try {
                        for (int tableId = 0; tableId < limit; tableId++) {
                            switch (tablesToSync[tableId]) {
                                case ObservedTableTracker.ADD:
                                    startTrackingTable(database, tableId);
                                    break;
                                case ObservedTableTracker.REMOVE:
                                    stopTrackingTable(database, tableId);
                                    break;
                            }
                        }
                        database.setTransactionSuccessful();
                    } finally {
                        database.endTransaction();
                    }
                    mObservedTableTracker.onSyncCompleted();
                } finally {
                    closeLock.unlock();
                }
            }
        } catch (IllegalStateException | SQLiteException exception) {
            // may happen if db is closed. just log.
            Log.e(Room.LOG_TAG, "Cannot run invalidation tracker. Is the db closed?",
                    exception);
        }
    }

the roomdata base catch SQLiteException exception, but the full class is
android.database.sqlite.SQLiteException
But when I use net.sqlcipher.database, will throw this exception net.sqlcipher.database.SQLiteException
It's different class. so Room can't try catch that. could you try to fix it?

Steps to Reproduce

  1. Use the RoomDataBase
  2. enableMultiInstanceInvalidation (Because we need multi process)

SQLCipher version (can be identified by executing PRAGMA cipher_version;):

SQLCipher for Android version:
4.4.3
Are you able to reproduce this issue within the SQLCipher for Android test suite?
I can't, just only firebase report

Note: If you are not posting a specific issue for the SQLCipher library, please post your question to the SQLCipher discuss site. Thanks!

@allenhsu-17live
Copy link
Author

like this pr

#556

@developernotes
Copy link
Member

Hi @allenhsu-17live

From your stack trace it appears the exception is thrown from the call to database.beginTransaction();, specifically here. Have you tried to create a new test case within the SQLCipher for Android test suite that showcases the behavior? Renaming the exception that is thrown doesn't resolve the root issue you're experiencing from what I can see.

@allenhsu-17live
Copy link
Author

Him @developernotes
I can't , because the report from online.
But I think we should consider use android.database.sqlite exception to replace all. right?

and even cause in this line database.beginTransaction();

I wrote the mock to test

    try {
        try {
            beginTransactionMock()
        } finally {
            closeLockMoc()
        }
    } catch (e: SQLException) {
        print("dosomething")
        //dosomething
    }
    
    fun beginTransactionMock() {
    throw SQLException("beginTransactionMock")
}

    fun closeLockMoc() {
    //    throw SQLException("closeLockMoc")
    }

The outermost try catch will still catch the error. will print dosomething
but if you use different packageNam

    try {
        try {
            beginTransactionMock()
        } finally {
            closeLockMoc()
        }
    } catch (e: SQLException) {
        print("dosomething")
        //dosomething
    }
    
    fun beginTransactionMock() {
    throw mock.SQLException("beginTransactionMock")
}

    fun closeLockMoc() {
    //    throw mock.SQLException("closeLockMoc")
    }

will be get crash

@developernotes
Copy link
Member

Hi @allenhsu-17live

I understand the catch logic you are suggesting, however, it doesn't address why the exception is being thrown in the first place. As to the rename, it may be possible to make an adjustment to the exception hierarchy without renaming. This is something we will consider.

@allenhsu-17live
Copy link
Author

@developernotes
Ok, got it,
But regardless of the root cause, I still suggest that you should use the official android exception instead of a copy. You can refer to the PR I mentioned

@stale
Copy link

stale bot commented Jul 21, 2021

Hello, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "bug", "enhancement", or "security" and I will leave it open. Thank you for your contributions.

@stale stale bot added the stale This issue lacks recent activity. label Jul 21, 2021
@stale
Copy link

stale bot commented Aug 22, 2021

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to reopen with up-to-date information.

@stale stale bot closed this as completed Aug 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This issue lacks recent activity.
Projects
None yet
Development

No branches or pull requests

2 participants