0

my code was

val cursor = getApplication<Application>().contentResolver.query(
    asksContract.CONTENT_URI, projection, null, null, sortOrder
)
dbCursor.setValue(cursor!!) // Update on different thread

And I changed it to

viewModelScope.launch(Dispatchers.IO) {
    val cursor = getApplication<Application>().contentResolver.query(
        TasksContract.CONTENT_URI, projection, null, null, sortOrder
    )
    dbCursor.setValue(cursor!!) 
    }

I think this is correct, no need to postValues as ime no longer in the main thread. Is this correct?

Also I am still getting warning

This `Cursor` should be freed up after use with `#close()`

Which makes sense, so should I

dbCursor.close()

after I setValue? or do I just ignore the error? The code is loading the data in a viewModel, the cursor is LiveData and there is an observer. I think maybe not as the cursor is still being used by the Recyclerview.

PS I setValues causes

java.lang.IllegalStateException: Cannot invoke setValue on a background thread

But starting a new thread withing the coroutine seems a bit silly, what have I missed?

The whole function is

private fun loadTasks() {
    val func = "loadTasks"
    Log.d(TAG, func)
    val projection = arrayOf(
        TasksContract.Columns.ID,
        TasksContract.Columns.TASK_NAME,
        TasksContract.Columns.TASK_DESCRIPTION,
        TasksContract.Columns.TASK_SORT_ORDER
    )
    val sortOrder =// set sortOrder so cursor is in correct order
        "${TasksContract.Columns.TASK_SORT_ORDER}, ${TasksContract.Columns.TASK_NAME}"
    viewModelScope.launch(Dispatchers.IO) {
        val cursor =
            getApplication<Application>().contentResolver.query(
                TasksContract.CONTENT_URI, projection, null, null, sortOrder
            )
        dbCursor.postValue(cursor!!) // runs setValue via Handler in MainThread
    }
    Log.d(TAG, "$func done")
}
Ben Edwards
  • 425
  • 5
  • 18
  • 1
    Sorry, my answer was incorrect--cursors are definitely *not* thread-safe. I've removed it. What kind of object is `dbCursor` exactly? `Cursor` doesn't have a `setValue()` function. – Tenfour04 Oct 18 '22 at 17:23
  • I just added full function to end of original post. – Ben Edwards Oct 18 '22 at 17:30
  • Also if I do cursor.close() after 'dbCursor.postValue(cursor!!) ' I get 'java.lang.IllegalStateException: attempt to re-open an already-closed object: SQLiteQuery: SELECT _id, Name, Description, SortOrder FROM Tasks ORDER BY SortOrder, Name'. Add as its a local varable, – Ben Edwards Oct 18 '22 at 17:33
  • 1
    I still don't understand where dbCursor comes from or what it is. Are `postValue` and `setValue` extension functions, and if so can we see what they look like? Storing a Cursor in a property or passing it around between functions is going to make it more tricky to correctly use it. Every cursor should be created, then immediately used in a try/finally block where finally closes it (or using Kotlin's `use` immediately after creating it) to ensure it is not leaked. If you want to use it again, you can't--you need to requery to get a new Cursor instance. – Tenfour04 Oct 18 '22 at 18:06
  • 1
    Regarding your initial question, using Dispatchers.IO won't work because there are many IO threads in that dispatcher. You have to work with the cursor on the exact same thread that returned its instance. – Tenfour04 Oct 18 '22 at 18:08
  • Sorry, me bad. The full code is at https://github.com/funkytwig/tasktimer/blob/master/app/src/main/java/com/funkytwig/tasktimer/TaskTimerViewModel.kt. dbcursor and cursor are defined nere top of TaskTimerViewModel class. – Ben Edwards Oct 19 '22 at 00:07
  • 1
    OK, all my confusion was from not knowing that was a LiveData rather than some specialized version of a Cursor. For the reasons in my long comment above, I believe it is extremely fragile to push Cursors through a LiveData for other classes to interact with. Unless it is an enormous amount of data returned in the query, I would try to convert it to a List of some data class in a `use` block and put that in the LiveData instead. If it is an enormous amount of data, I'm not sure of the best solution. Maybe a subclass of MutableLiveData that can close the previous value before replacing it. – Tenfour04 Oct 19 '22 at 01:26
  • Thanks for this. I'm fairly new to Android but I think I get it and is something I will come back to after competing for the course. Basically, rather than use a cursor you use a list. You use a cursor to update the list and that way you can close it. Am I correct in thinking with cursors only fetch the data from the DB as it is needed, rather than all of it, hence the comment about large datasets? – Ben Edwards Oct 21 '22 at 12:22
  • 1
    Yes, Cursor saves RAM by not loading the entire results of a query into memory. Only chunks at a time. If your result set is extremely large, you may want to avoid copying it to a List and try to use the Cursor. More information here: https://stackoverflow.com/questions/31465069/does-android-sqlite-cursor-load-all-records-into-memory-at-once – Tenfour04 Oct 21 '22 at 13:36

1 Answers1

0

use try-finally block

finally {
   bCursor.close()
}
Vera Iureva
  • 344
  • 1
  • 9