Conversation
Track whether the logger batch processor has accepted an item and skip empty flush and restart-close scheduling until then. This prevents SDK initialization and Android background callbacks from starting a worker thread when Logs are unused. Co-Authored-By: Claude <noreply@anthropic.com>
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
runningcode
left a comment
runningcode
left a comment
There was a problem hiding this comment.
This seems like a bugfix unrelated to the stack. Can we make this an independent PR?
Before this PR stack we used noop instances to avoid the thread creation. Since we can't rely on enableLogs / enableMetrics anymore this PR is needed. It's not a separate bugfix but rather an optimization that should be part of the PR stack. |
| @@ -114,6 +116,9 @@ private void maybeSchedule(boolean immediately) { | |||
|
|
|||
| @Override | |||
| public void flush(long timeoutMillis) { | |||
There was a problem hiding this comment.
Bug: A race condition between add() and flush() can cause flush() to return early without waiting for a queued item, breaking its synchronous contract.
Severity: MEDIUM
Suggested Fix
To fix the race condition, set the hasAcceptedItem flag to true before adding the event to the queue with queue.offer(logEvent). Alternatively, use a lock or another synchronization primitive to protect the critical section between adding to the queue and setting the flag.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java#L118
Potential issue: A race condition exists between the `add()` and `flush()` methods in
`LoggerBatchProcessor`. A thread can add an item to the `queue` via
`queue.offer(logEvent)`, but before it sets the `hasAcceptedItem` flag to `true`,
another thread can call `flush()`. The `flush()` method checks `if (!hasAcceptedItem)`
and returns immediately, failing to wait for the newly queued item. This violates the
implicit contract of `flush()`, which is expected to block until all pending items are
processed, especially during client shutdown when `SentryClient.close()` relies on it to
ensure all logs are sent.
Also affects:
sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java:88~89
This branch has not been deployed
2 participants
PR Stack (Logs and Metrics Enable Flags)
📜 Description
Tracks whether
LoggerBatchProcessorhas accepted its first Log item. Before that point, empty flushes and restart closes do not schedule processor work. Normal close still closes the executor directly.After the first accepted item, batching, flushing, and restart-close behavior remain unchanged. Items rejected because of shutdown or queue capacity do not mark the processor as used.
💡 Motivation and Context
The aggregate Logs enable flag has been removed, so every SDK client now owns a logger batch processor. Without this guard, lifecycle flushes—particularly Android background callbacks—can start a worker thread even when the application never captures a Log.
💚 How did you test it?
./gradlew :sentry:test :sentry-android-core:testReleaseUnitTest./gradlew spotlessApply apiDump📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
Remove the aggregate Metrics enable flag.
#skip-changelog