[issues/93] Expand lock scope to ensure thread safety and prevent race conditions#125
[issues/93] Expand lock scope to ensure thread safety and prevent race conditions#125dochoi-bot wants to merge 1 commit intodimitris-c:mainfrom
Conversation
|
|
||
| rendererContext.lock.lock() | ||
| bufferContext.frameStartIndex = (bufferContext.frameStartIndex + totalFramesCopied) % bufferContext.totalFrameCount | ||
| bufferContext.frameUsedCount -= totalFramesCopied |
There was a problem hiding this comment.
| bufferContext.frameUsedCount -= totalFramesCopied | |
| if bufferContext.frameUsedCount >= totalFramesCopied { | |
| bufferContext.frameUsedCount -= totalFramesCopied | |
| } else { | |
| bufferContext.frameUsedCount = 0 | |
| } |
As another option,
What are your thoughts on adding overflow protection here and at line 124?
|
Expanding the lock scope is not ideal, even the locks I have shouldn't be there because the audio thread is a high priority and will not "wait" for a lock to get unlocked. So if you expand the lock you might hit more issues... Ideally I would have to move into a lock-less ring-buffer in order to properly handle this, but this is needs time and testing |
|
@dimitris-c I understand it’s not the ideal long-term solution compared to a proper lock-free ring buffer, but I’m wondering if it’s acceptable as a short-term mitigation until we refactor the buffer logic. |
|
I will try to add this in on a separate fix, thanks - most likely I'll add a temporary solution, thanks for the PR — |
#93 (comment)
It seems to be causing a race condition with the resetBuffers function.
Are there any potential risks with this implementation?