Prevent concurrent modifications of audit log

[seadowg]

Fixes this crash.

This makes sure that logEvent and flush are never executed concurrently which could cause problems.

Why is this the best possible solution? Were any other approaches considered?

We should also remove all uses of logEvent and flush on the UI thread, but that's part of ongoing ANR work (#5867).

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

It'll be hard to reproduce the linked crash (or we probably would have caught it ourselves). I think the best thing to test here is general audit log behaviour - I've made some changes here to how events are logged and that could be risky.

Do we need any specific form for testing your changes? If so, please attach one.

Any forms with audit enabled.

Before submitting this PR, please make sure you have:

• added or modified tests for any new or changed behavior
• run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
• added a comment above any new strings describing it for translators
• verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• verified that any new UI elements use theme colors. UI Components Style guidelines

[dbemke]

In audit logs the "location providers enabled" event is missing after turning it on in the middle of filling a form (not missing in the store version).
The latitude, longitude and accuracy are present in the events after turning on the location permission.

Steps to reproduce:

1. Location permission is granted to Collect.
2. Turn off the location permission of the device in settings.
3. Go to the AuditTestLocationBackground.xml form.
   AuditTestLocationBackground.xml.txt
4. Choose “cancel” in the dialog asking to enable GPS.
5. Fill in some questions.
6. Minimize the app.
7. Go to device settings.
8. Turn on the location permission.
9. Go back to the opened form and fill some questions.
10. Click “Save as draft” button (end page).

Expected behavior
In audit log after turning on the location permission and going back to the form there should be "location providers enabled" event.

[seadowg]

Turn off the location permission of the device in settings.

Just to check: is this disabling "Use location" for all apps, or just removing the location permission from Collect?

EDIT: I realise that "location providers enabled" implies the former.

[seadowg]

@dbemke I'm not able to reproduce on an Android 13 device. Are you seeing this sporadically or only on a specific OS/device?

[dbemke]

I think I know what might have happened. It also appeared on Android 13. I just checked Android 14 and it didn't occur.
I mainly tested it on Android 10 when comparing results and I'm able to reproduce it every time. Now I managed to get the event "location provider enabled" but I left the form opened for 5min after enabling the location again. So it seems that in the store version of Collect the event appeared faster. While testing I usually wait 10-20seconds after changing location permission so in that case I got different results (the event present or not). I guess that geometry values that were in audit logs might have been some stored values but really the location wasn't enabled during that 20second gap. Do you think this might be the case? Should I check the difference in time after enabling location and seeing the event in the audit logs or it's an edge case and it can stay as it is?

[dbemke]

@seadowg We finished testing audit logs and haven't found any other issues so if you decide that the issue mentioned above is ok, could you remove the "needs testing" label?

[seadowg]

Should I check the difference in time after enabling location and seeing the event in the audit logs or it's an edge case and it can stay as it is?

Yeah that'd be great! It feels to me like this probably hasn't actually changed between versions and we got unlucky testing it.

[dbemke]

Today I tested it again and now the event is present. I'll test the event separately so that the PR can be closed.

[dbemke]

Tested with Success!

Verified on Android 10

Verified cases:

• regression checks in audit logs

[srujner]

Tested with Success!