Skip to content

Fix small memory leak and mark more exceptions #177

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

Conversation

brody4hire
Copy link

Fixed small memory leak

There is a very small memory leak in the code that was inherited from the Android project as reported here: https://code.google.com/p/android/issues/detail?id=22794

The Android project already fixed this leak when making some other changes in November 2011, in this commit: aosp-mirror/platform_frameworks_base@e5360fb

The first commit in this PR fixes the memory leak by replacing SQLiteDatabase.ActiveDatabases with private static WeakHashMap<SQLiteDatabase, Object> sActiveDatabases and cleaning it up in SQLiteDatabase.onAllReferencesReleased().

Note that in the Android project the reference is not cleaned up in case the SQLiteDatabase instance is finalized, while no such distinction is made in my commit. I suspect their reasoning was that it would not be worth while to cleanup a weak reference to a SQLiteDatabase instance that is known to be going away.

Mark some more exceptions in SQLiteDatabase class

I marked some more exceptions that may be thrown, as discussed in #176. I also added some Javadoc comments to public functions where missing.

Small fix to public SQLiteDatabase.yieldIfContended functions

I added a small fix to SQLiteDatabase.yieldIfContended(), SQLiteDatabase.yieldIfContendedSafely(), and SQLiteDatabase.yieldIfContendedSafely(long) to simply return false if the database is closed.

Testing

I tested the changes in a version of the test suite that repeats 9 times, as described in sqlcipher/sqlcipher-android-tests#8.

Chris Brody added 2 commits June 2, 2015 17:36
by replacing it with private static WeakHashMap<SQLiteDatabase, Object> sActiveDatabases and
cleaning it up in SQLiteDatabase.onAllReferencesReleased(), following what was done in:
aosp-mirror/platform_frameworks_base@e5360fb

NOTE: in the Android project the reference is _not_ cleaned up in case the SQLiteDatabase instance is finalized.
No such distinction is made in this commit.
…LiteDatabase.yieldIfContended* against closed database; some missing Javadoc in SQLiteDatabase.java
@developernotes developernotes merged commit 98de842 into sqlcipher:master Jun 2, 2015
@developernotes
Copy link
Member

@brodybits - this is great, thanks!

@brody4hire brody4hire deleted the fix-small-memleak-and-mark-more-exceptions branch March 19, 2019 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants