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

UPSTREAM: docker/distribution: 2299: Fix signalling Wait in regulator.enter #14581

Merged
merged 1 commit into from
Jun 13, 2017

Conversation

dmage
Copy link
Contributor

@dmage dmage commented Jun 12, 2017

In some conditions, regulator.exit may not send a signal to blocked
regulator.enter.

Let's assume we are in the critical section of regulator.exit and r.available
is equal to 0. And there are three more gorotines. One goroutine also executes
regulator.exit and waits for the lock. Rest run regulator.enter and wait for
the signal.

We send the signal, and after releasing the lock, there will be lock
contention:

  1. Wait from regulator.enter
  2. Lock from regulator.exit

If the winner is Lock from regulator.exit, we will not send another signal to
unlock the second Wait.

….enter

In some conditions, regulator.exit may not send a signal to blocked
regulator.enter.

Let's assume we are in the critical section of regulator.exit and r.available
is equal to 0. And there are three more gorotines. One goroutine also executes
regulator.exit and waits for the lock. Rest run regulator.enter and wait for
the signal.

We send the signal, and after releasing the lock, there will be lock
contention:

  1. Wait from regulator.enter
  2. Lock from regulator.exit

If the winner is Lock from regulator.exit, we will not send another signal to
unlock the second Wait.

Signed-off-by: Oleg Bulatov <obulatov@redhat.com>
@mfojtik
Copy link
Contributor

mfojtik commented Jun 12, 2017

[test]

@mfojtik
Copy link
Contributor

mfojtik commented Jun 12, 2017

@miminar or @legionus for review?

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to f4d649e

@miminar
Copy link

miminar commented Jun 12, 2017

Good job! This resolves https://bugzilla.redhat.com/show_bug.cgi?id=1436841

I couldn't find the problem in the faulty signaling. Thanks!

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2125/) (Base Commit: c09c601)

Copy link
Contributor

@legionus legionus left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@miminar miminar left a comment

Choose a reason for hiding this comment

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

LGTM

@mfojtik
Copy link
Contributor

mfojtik commented Jun 13, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to f4d649e

@openshift-bot
Copy link
Contributor

openshift-bot commented Jun 13, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/985/) (Base Commit: 599a720) (Image: devenv-rhel7_6350)

@openshift-bot openshift-bot merged commit 89fb136 into openshift:master Jun 13, 2017
@miminar
Copy link

miminar commented Jun 26, 2017

And the upstream PR has been merged! Congrats.

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.

5 participants