Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++] Fixed race condition in MemoryLimitController #10142

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

merlimat
Copy link
Contributor

@merlimat merlimat commented Apr 6, 2021

Motivation

There is a race condition between the reserveMemory() and the mutex condition notification done when the other thread releases the memory.

The problem is that we're doing tryReserveMemory() in a loop but without holding the mutex. It can happen that we tryReserveMemory() return false (memory is full) and before we acquire the mutex and start waiting on the condition, the other thread had already released the memory and tried to notify.

Modification

We need to do the tryReserveMemory() while holding the mutex, so that we don't have this condition. It is fine to do it without mutex the first time, so that we can avoid the mutex contention in the normal case, as long as we fall back to the mutex and check again.

@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Apr 6, 2021
@merlimat merlimat added this to the 2.8.0 milestone Apr 6, 2021
@merlimat merlimat self-assigned this Apr 6, 2021
merlimat added a commit to merlimat/pulsar that referenced this pull request Apr 6, 2021
@jerrypeng jerrypeng merged commit 5c521a3 into apache:master Apr 9, 2021
@merlimat merlimat deleted the fix-cpp-memory-controller branch April 9, 2021 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants