collect
collect copied to clipboard
Allow logging to audit log from background threads
Currently, using AuditEventLogger
to log events from a background thread is dangerous as it could cause a ConcurrentModificationException
due to the underlying ArrayList
that events are stored in. After #5259, logging from a background thread will be protected by restricting logging to the UI thread (similar to how IO explodes when it runs on the UI thread in Android).
We might want to look at making our audit logging "thread safe" to prevent these kinds of problems. By that I mean:
- Events end up in the log in the order that
AuditEventLogger#logEvent/flush
is called - Events added by
AuditEventLogger#logEvent/flush
always end up in the log (none are dropped/lost)
Additionally, we want to avoid the UI thread having to wait for another AuditEventLogger#logEvent/finalize
call to complete.
As I'm writing this out, it does strike me that we may not actually want to ever call AuditEventLogger#logEvent/flush
from a background thread: even if condition 1 is true, if we're calling from a background thread we will lose control of the order in which logEvent/finalize
is called. We might simply be passing the concurrency problem up the call stack by doing this. If that's the case, it might be worth keeping the UI thread check we have and potentially just adding async writes so that we don't block the UI thread when logging/flushing like we do now.
Either way, this would be good to discuss!
As part of this train of thought, I think we also want to think about moving logEvent/flush
calls closer to the UI interactions they represent. For example, currently we log question
events when rendering questions, rather than when the user switches between screens creating a pretty risky tie between rendering and the audit log. I might be forced to address this in my fix for #5240.