namespace: Copy ttrpc metadata before setting header#3506
namespace: Copy ttrpc metadata before setting header#3506estesp merged 1 commit intocontainerd:masterfrom
Conversation
|
Build succeeded.
|
darfux
force-pushed
the
copy_md_when_set_ttrpcheader
branch
from
16e16d4 to
d18ae68
Compare
|
Build succeeded.
|
Codecov Report
@@ Coverage Diff @@
## master #3506 +/- ##
==========================================
+ Coverage 44.26% 44.27% +0.01%
==========================================
Files 124 124
Lines 13675 13683 +8
==========================================
+ Hits 6053 6058 +5
- Misses 6687 6689 +2
- Partials 935 936 +1
Continue to review full report at Codecov.
|
|
@darfux @crosbymichael depends on |
|
I think these maps will be very small, either work in this case |
|
In this case I think we should copy slices as well since we're going to append to it (concurrently) later. This is what GRPC does as well: https://github.com/containerd/containerd/blob/master/vendor/google.golang.org/grpc/metadata/metadata.go#L130 |
|
Ahh, good catch, we should copy the slice |
darfux
force-pushed
the
copy_md_when_set_ttrpcheader
branch
from
d18ae68 to
85ee8f8
Compare
|
Build succeeded.
|
darfux
force-pushed
the
copy_md_when_set_ttrpcheader
branch
from
85ee8f8 to
7814bfb
Compare
|
Build succeeded.
|
If there are multiple goroutines calling `namespace.WithNamespace` on a ctx that already had namespace, there will be a data race when `withTTRPCNamespaceHeader` calling `MD.Set()`. So we have to copy the md before using it. Signed-off-by: Li Yuxuan <[email protected]>
darfux
force-pushed
the
copy_md_when_set_ttrpcheader
branch
from
7814bfb to
a3a3063
Compare
darfux
changed the title
|
Build succeeded.
|
|
I've updated the commit and added test cases, PTAL:) @fuweid @mxpv @crosbymichael |
6 participants
If there are multiple goroutines calling
namespace.WithNamespaceon actx that already had namespace, there will be a data race when
withTTRPCNamespaceHeadercallingMD.Set(). So we have to copy the mdbefore using it.
Fixes #3504
Signed-off-by: Li Yuxuan [email protected]