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

Fix deadlock caused by timeout overflow #3998

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

haozheng-cobalt
Copy link
Contributor

b/346812667

b/346812667
Change-Id: I9fd9ba7652a1aaee9d748c2dee60cb57b59d4cda

Change-Id: I48e03ebc80e441619a0131dd62917016b986f925
@haozheng-cobalt haozheng-cobalt self-assigned this Aug 15, 2024
@haozheng-cobalt haozheng-cobalt enabled auto-merge (squash) August 15, 2024 17:10
@haozheng-cobalt haozheng-cobalt merged commit abc3419 into youtube:main Aug 15, 2024
321 of 322 checks passed
@kaidokert kaidokert added the cp-25.lts.1+ Cherry Pick to the 25.lts.1+ branch label Sep 21, 2024
cobalt-github-releaser-bot pushed a commit that referenced this pull request Sep 21, 2024
b/346812667

(cherry picked from commit abc3419)
kaidokert pushed a commit that referenced this pull request Sep 21, 2024
Refer to the original PR: #3998

b/346812667
b/368560498
b/368244422

Co-authored-by: Hao <[email protected]>
@@ -66,13 +66,24 @@ bool ConditionVariable::WaitTimed(int64_t duration) const {
bool was_signaled = SbConditionVariableIsSignaled(
SbConditionVariableWaitTimed(&condition_, mutex_->mutex(), duration));
#else
if (duration < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

This appears to address a symptom of a bug outside of this function.

If this ever is negative, then the caller is doing something wrong. Adding additional checks here makes every call to this function slower, and it is part of the critical path on some places where performance is critical. I would suggest making the fix outside of this function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cp-25.lts.1+ Cherry Pick to the 25.lts.1+ branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants