[ODK Developers] Unsaved answers lost when tablet device goes to sleep

Hi Mark and all,

I've tracked this down to the onPause method in FormEntryActivity, which
is called when the device goes to sleep. This method attemps to save
the answers but the call to saveAnswersForCurrentScreen is guarded by
the following nested ifs:

if (mSaveToDiskTask != null && mSaveToDiskTask.getStatus() !=
AsyncTask.Status.RUNNING) {
if (mCurrentView != null && formController != null &&
formController.currentPromptIsQuestion()) {

What I'm seeing is mSaveToDiskTask is null so
saveAnswersForCurrentScreen is never called hence the answers aren't
saved to be restored.

My question is why are these nested if statements neccessary? When a
user swipes to move to the next or previous page, showNextView and
showPreviousView don't uses these same ifs. They only test for
formController.currentPromptIsQuestion(). Is there some reason that
onPause needs the extra checks that I don't understand?

Perhaps this is an attempt to prevent a race condition where
mSaveToDiskTask is trying to save the form and we also try to save the
form at the same time, which could cause a crash.

If so, then the if condition is messed up: it should be

if (mSaveToDiskTask == null || mSaveToDiskTask.getStatus() !=
AsyncTask.Status.RUNNING)

If I remove the if statements from onPause and just call
saveAnswersForCurrentScreen, the answers for the current page are
restored when the device is awaken.

If this is a race condition, that would work 99% of the time.

And if it's actually serious to lose the race, then we should guard moving
to the next page in the same way.

Cheers, Chris.

··· On Thu, 25 Oct 2012, mark.devol@us.army.mil wrote: -- Aptivate | http://www.aptivate.org | Phone: +44 1223 967 838 Future Business, Cam City FC, Milton Rd, Cambridge, CB4 1UY, UK

Aptivate is a not-for-profit company registered in England and Wales
with company number 04980791.

OK. I've changed the condition to be:

    if (mSaveToDiskTask == null || mSaveToDiskTask.getStatus() ==

AsyncTask.Status.FINISHED) {

This is in the rev 1020 release.

The race condition we are trying to avoid is if you are saving the data
from the UI elements into the in-memory data model
(saveAnswersForCurrentScreen) at the same time there is a background task
(thread) writing that data model to the xml file on the SDcard
(SaveToDiskTask).

The above condition ensures that either there is no background task or the
task is finished. Then, if we are on a view and have a valid formController
object, and are on a page that shows one or more questions, we will save
the values from the UI elements into the data model.

This method has an extra check on the mCurrentView because it can be called
during the presentation of the form's title page and the finalize screen.
It has an extra check on the formController because there are different
timings on the 2.x and 4.x Androids allowing for a flow path where the
formController may be null on entry to this method.

Mitch

··· On Fri, Oct 26, 2012 at 1:32 AM, Chris Wilson wrote:

Hi Mark and all,

On Thu, 25 Oct 2012, mark.devol@us.army.mil wrote:

I've tracked this down to the onPause method in FormEntryActivity, which

is called when the device goes to sleep. This method attemps to save the
answers but the call to saveAnswersForCurrentScreen is guarded by the
following nested ifs:

if (mSaveToDiskTask != null && mSaveToDiskTask.getStatus() !=
AsyncTask.Status.RUNNING) {
if (mCurrentView != null && formController != null &&
formController.**currentPromptIsQuestion()) {

What I'm seeing is mSaveToDiskTask is null so saveAnswersForCurrentScreen
is never called hence the answers aren't saved to be restored.

My question is why are these nested if statements neccessary? When a
user swipes to move to the next or previous page, showNextView and
showPreviousView don't uses these same ifs. They only test for
formController.**currentPromptIsQuestion(). Is there some reason that
onPause needs the extra checks that I don't understand?

Perhaps this is an attempt to prevent a race condition where
mSaveToDiskTask is trying to save the form and we also try to save the form
at the same time, which could cause a crash.

If so, then the if condition is messed up: it should be

if (mSaveToDiskTask == null || mSaveToDiskTask.getStatus() !=
AsyncTask.Status.RUNNING)

If I remove the if statements from onPause and just call

saveAnswersForCurrentScreen, the answers for the current page are restored
when the device is awaken.

If this is a race condition, that would work 99% of the time.

And if it's actually serious to lose the race, then we should guard moving
to the next page in the same way.

Cheers, Chris.

Aptivate | http://www.aptivate.org | Phone: +44 1223 967 838
Future Business, Cam City FC, Milton Rd, Cambridge, CB4 1UY, UK

Aptivate is a not-for-profit company registered in England and Wales
with company number 04980791.

--
Mitch Sundt
Software Engineer
University of Washington
mitchellsundt@gmail.com