Skip to content

bugfix: fix potential thread-safety issue in BacktraceLogManager - #296

Merged
KishanPRao merged 2 commits into
masterfrom
bugfix/lock-log-manager
Jul 10, 2026
Merged

bugfix: fix potential thread-safety issue in BacktraceLogManager#296
KishanPRao merged 2 commits into
masterfrom
bugfix/lock-log-manager

Conversation

@KishanPRao

@KishanPRao KishanPRao commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

ToSourceCode() reads the log queue without the lock that Enqueue()/Dequeue() use, so a concurrent log from a background thread could corrupt the read. This makes the manager thread-safe on its own instead of relying on the caller to lock.

Changes

  • BacktraceLogManager: Wrap the LogQueue read in ToSourceCode() with lockObject.

ref: BT-7318

@KishanPRao
KishanPRao requested a review from melekr July 9, 2026 17:04
@KishanPRao KishanPRao self-assigned this Jul 9, 2026
@KishanPRao KishanPRao mentioned this pull request Jul 9, 2026

@melekr melekr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: the lock scope could be narrowed to only the ToArray() copy, then build the StringBuilder after releasing the lock. Still looks good given the small default log count, but the narrower scope would be cleaner.

@melekr
melekr marked this pull request as ready for review July 9, 2026 20:52
@KishanPRao

Copy link
Copy Markdown
Contributor Author

Non-blocking: the lock scope could be narrowed to only the ToArray() copy, then build the StringBuilder after releasing the lock. Still looks good given the small default log count, but the narrower scope would be cleaner.

Good idea, I've narrowed the lock's scope.

@KishanPRao
KishanPRao merged commit 1cfe60b into master Jul 10, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants