fix: modify lock location of exec delete avoid exec hang#2624
fix: modify lock location of exec delete avoid exec hang#2624estesp merged 1 commit intocontainerd:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2624 +/- ##
=======================================
Coverage 44.05% 44.05%
=======================================
Files 94 94
Lines 10238 10238
=======================================
Hits 4510 4510
Misses 5008 5008
Partials 720 720
Continue to review full report at Codecov.
|
runtime/v1/linux/proc/exec.go
Outdated
There was a problem hiding this comment.
what if we removed it from here and just applied the lock around the s.transition() calls in the Delete methods?
There was a problem hiding this comment.
Yes, s.transition("deleted") need apply lock, but I just wonder whether delete need a lock. And update as you said.
func (e *execProcess) delete(ctx context.Context) error {
e.wg.Wait()
...
}
delete exec process will wait for io copy finish, if wait here,
other process can not get lock of shim service.
1. apply lock around s.transition() calls in the Delete methods.
2. put lock after wait io copy in exec Delete.
Signed-off-by: Ace-Tang <aceapril@126.com>
721829f to
079292e
Compare
|
LGTM |
|
I think this can help fix #2624. There might still be process/goroutine leakage, but at least won't make containerd-shim hang. Should we cherry-pick this into 1.1? |
[release/1.1] Backport: modify lock location of exec delete avoid exec hang
func (e *execProcess) delete(ctx context.Context) error {
e.wg.Wait()
...
}
delete exec process will wait for io copy finish, if wait here,
other process can not get lock of shim service.
Signed-off-by: Ace-Tang aceapril@126.com
how to produce the problem
sudo ctr run -d docker.io/library/busybox:latest b1 tope1finish.also
ctr t lswill also hang, since it also need acquire shim lock to get task status.