RUMM-2202 Ensure thread safety#936
Conversation
f69c36a to
5c4a506
Compare
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
LGTM! I've left a suggestion in the test and a question regarding the TODO item, but overall it is non-blocking.
| import androidx.annotation.UiThread | ||
|
|
||
| class Foo { | ||
| @UiThread |
There was a problem hiding this comment.
| import androidx.annotation.UiThread | |
| class Foo { | |
| @UiThread | |
| import androidx.annotation.WorkerThread | |
| class Foo { | |
| @WorkerThread |
|
|
||
| package com.datadog.android.v2.core.internal.storage | ||
|
|
||
| internal fun interface BatchConfirmation { |
There was a problem hiding this comment.
are we removing functional interface property here (and in the other interfaces), because it is complicated to get annotation in case of lamba implementation of the functional interface?
| @Suppress("ThreadSafety") // Crash handling, can't delegate to another thread | ||
| rootScope.handleEvent(event, writer) | ||
| } else if (event is RumRawEvent.SendTelemetry) { | ||
| @Suppress("ThreadSafety") // TODO RUMM-1503 delegate to another thread |
There was a problem hiding this comment.
Do we need to do it here? I mean generally speaking yes (because we known only the interface of the writer, but not implementation), but if we are talking about this particular case then normally writer is a ScheduledWriter, so when writer.write is called from the TelemetryEventHandler, write operation will be performed on the worker thread (because ScheduledWriter bundles ExecutorService).
So in the end we delegate this call to the worker thread for nothing.
There was a problem hiding this comment.
The thing here is to provide a way to enforce the thread safety, and maybe we can find another way to make sure the write operation is called in the right thread. Maybe the scheduled writer needs to use a different interface with an @AnyThread annotation? Anyway this will be handled in the TODO task.
5c4a506 to
5cc14b3
Compare
3 participants
What does this PR do?
@WorkerThread/@UiThread…