Skip to content

Merge Container and State mutexes#7775

Merged
vieux merged 1 commit intomoby:masterfrom
LK4D4:merge_state_lock
Aug 29, 2014
Merged

Merge Container and State mutexes#7775
vieux merged 1 commit intomoby:masterfrom
LK4D4:merge_state_lock

Conversation

@LK4D4
Copy link
Copy Markdown
Contributor

@LK4D4 LK4D4 commented Aug 28, 2014

Resolved all deadlocks and fixed race between kill and monitor.resetContainer

I'm not fully embedded State now, because it'll be huge and not very pleasant for rebase diff.

Fixes #7600

@crosbymichael
Copy link
Copy Markdown
Contributor

crosbymichael commented Aug 28, 2014

Can you reference the issues that this fixes?

@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Aug 28, 2014

@crosbymichael I'm not sure that there is issues about this. This fixes cases when we can change state even if container locked.

@crosbymichael
Copy link
Copy Markdown
Contributor

crosbymichael commented Aug 28, 2014

There is the zombie issue right?

@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Aug 28, 2014

I think that we decided, that that issue (#7600) will be resolved by lock on whole Stop method and I think that @vincentwoo will prepare PR. So, this is PR is just for error prevention on locks.

@vincentwoo
Copy link
Copy Markdown
Contributor

vincentwoo commented Aug 28, 2014

Sounds good. Should I build off this commit?

@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Aug 28, 2014

@vincentwoo You can try :)

@crosbymichael
Copy link
Copy Markdown
Contributor

crosbymichael commented Aug 28, 2014

LGTM

@vincentwoo
Copy link
Copy Markdown
Contributor

vincentwoo commented Aug 29, 2014

Upon further testing, this PR does appear to fix #7600. However, a new issue has cropped up. When trying to start a container and remove(force: true) at the same time, you will get some remove failures with this error: Driver aufs failed to remove root filesystem.

The containers DO appear to be properly removed, though - the processes are gone and the mount directory is not present. I think this is at least good enough to merge now.

@LK4D4
Merge Container and State mutexes
517ba44 
Resolved all deadlocks and fixed race between kill and
monitor.resetContainer
Fixes #7600

Signed-off-by: Alexandr Morozov <[email protected]>
@LK4D4
Copy link
Copy Markdown
Contributor Author

LK4D4 commented Aug 29, 2014

@crosbymichael I added "Fixes #7600" to commit message. So feel free to merge if all okay for you.

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Aug 29, 2014

LGTM

vieux added a commit that referenced this pull request Aug 29, 2014
@vieux
Merge pull request #7775 from LK4D4/merge_state_lock
280b64b 
Merge Container and State mutexes
@vieux vieux merged commit 280b64b into moby:master Aug 29, 2014
@LK4D4 LK4D4 mentioned this pull request Aug 30, 2014
Closed
@LK4D4 LK4D4 deleted the merge_state_lock branch August 30, 2014 08:33
@tiborvass tiborvass mentioned this pull request Sep 5, 2014
Merged
@thaJeztah thaJeztah mentioned this pull request Oct 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Very frequently getting zombie processes from killed Docker containers

4 participants

@LK4D4 @crosbymichael @vincentwoo @vieux