Shim locking improvements and context cancel#2589
Shim locking improvements and context cancel#2589crosbymichael merged 2 commits intocontainerd:masterfrom
Conversation
crosbymichael
force-pushed
the
shim-robo
branch
from
1ed999e to
451447f
Compare
Codecov Report
@@ Coverage Diff @@
## master #2589 +/- ##
==========================================
+ Coverage 40.97% 44.05% +3.08%
==========================================
Files 69 98 +29
Lines 8350 10130 +1780
==========================================
+ Hits 3421 4463 +1042
- Misses 4385 4945 +560
- Partials 544 722 +178
Continue to review full report at Codecov.
|
crosbymichael
changed the title
|
As long as Anyway, this is still better than before. :) |
|
@Random-Liu process is threadsafe. And this is the start of various fixes. I'm getting timeouts working for ttrpc clients, etc |
|
@crosbymichael SGTM. Thanks a lot! :) |
Signed-off-by: Michael Crosby <[email protected]>
There was a problem hiding this comment.
nit: could avoid the defer cost here
Only lock around shim state and not on actions Signed-off-by: Michael Crosby <[email protected]>
crosbymichael
force-pushed
the
shim-robo
branch
from
451447f to
2205e8d
Compare
| n, err := unix.EpollWait(e.fd, events[:], -1) | ||
| if err != nil { | ||
| if err == unix.EINTR { | ||
| continue |
There was a problem hiding this comment.
Probably OK since this is the last item in the select, but may want to make sure this continues the loop rather than the select.
There was a problem hiding this comment.
Actually, with the 2nd for loop below, this may or may not be working as expected.
There was a problem hiding this comment.
you break out of selects, you continue for loops
|
LGTM |
| if err != nil { | ||
| return nil, errdefs.ToGRPC(err) | ||
| } | ||
| s.mu.Lock() |
There was a problem hiding this comment.
Should this check existence again after the lock is reacquired or is that condition not possible?
7 participants
This improves shim locking where we only hold the lock for shim state, and not system level operations.