Skip to content

fix: avoid modifying source during resolver.Copy #2230

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 1, 2024
Merged

fix: avoid modifying source during resolver.Copy #2230

merged 1 commit into from
Feb 1, 2024

Conversation

jedevc
Copy link
Collaborator

@jedevc jedevc commented Jan 31, 2024

Fixes #2229

It looks like this case could only happen with multiple sources? But regardless, we definitely shouldn't be modifying a source input, we should instead make a clone.

@jedevc
fix: avoid modifying source during resolver.Copy
95bdecc 
Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc requested a review from crazy-max January 31, 2024 14:45
This was referenced Jan 31, 2024
Closed
Closed
@dunglas
Copy link

dunglas commented Jan 31, 2024

I tried it in FrankenPHP's CI and it looks like it triggers another bug:

[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x11e7f95]

goroutine 1985 [running]:
go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace.(*clientTracer).end(0xc005c1da40, {0x23169ef, 0xc}, {0x0?, 0x0?}, {0xc006e63200?, 0x26a2980?, 0x4})
	go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace@v0.45.0/clienttrace.go:231 +0x795
go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace.(*clientTracer).gotConn(0x2699d58?, {{0x26a2980?, 0xc0057e7500?}, 0x0?, 0x50?, 0x3?})
	go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace@v0.45.0/clienttrace.go:288 +0x64d
net/http.http2traceGotConn(0xc00626e1a0?, 0xc006a74c00, 0x1)
	net/http/h2_bundle.go:10239 +0x1dd
net/http.(*http2Transport).RoundTripOpt(0xc000350510, 0xc002930000, {0x40?})
	net/http/h2_bundle.go:7648 +0x19c
net/http.(*http2Transport).RoundTrip(...)
	net/http/h2_bundle.go:7598
net/http.http2noDialH2RoundTripper.RoundTrip({0x3850580?}, 0xc002930000?)
	net/http/h2_bundle.go:10203 +0x16
net/http.(*Transport).roundTrip(0x3850580, 0xc002930000)
	net/http/transport.go:549 +0x39e
net/http.(*Transport).RoundTrip(0x3850300?, 0x2699d58?)
	net/http/roundtrip.go:17 +0x13
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.(*Transport).RoundTrip(0xc000343880, 0xc002a51c00)
	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp@v0.45.0/transport.go:116 +0x52b
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.(*Transport).RoundTrip(0xc002a26fc0, 0xc002a51b00)
	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp@v0.45.0/transport.go:116 +0x52b
net/http.send(0xc002a51b00, {0x2675de0, 0xc002a26fc0}, {0xc0043fe301?, 0xc001a3a5e0?, 0x0?})
	net/http/client.go:260 +0x606
net/http.(*Client).send(0xc002a60210, 0xc002a51b00, {0x7f9e18ebb888?, 0xc006aa9d30?, 0x0?})
	net/http/client.go:181 +0x98
net/http.(*Client).do(0xc002a60210, 0xc002a51b00)
	net/http/client.go:724 +0x912
net/http.(*Client).Do(...)
	net/http/client.go:590
github.com/containerd/containerd/remotes/docker.(*request).do(0xc001a2c900, {0x2699d58, 0xc001a421b0})
	github.com/containerd/containerd@v1.7.12/remotes/docker/resolver.go:591 +0x4e5
github.com/containerd/containerd/remotes/docker.(*request).doWithRetries(0xc001958fc0?, {0x2699d58, 0xc001a421b0}, {0x0, 0x0, 0x0})
	github.com/containerd/containerd@v1.7.12/remotes/docker/resolver.go:600 +0x45
github.com/containerd/containerd/remotes/docker.dockerPusher.push({0xc0001722d0, {0xc000776619, 0x18}, {0x7f9e18f094c8, 0xc00065e6c0}}, {0x2699d58, 0xc001a42180}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, ...}, ...}, ...)
	github.com/containerd/containerd@v1.7.12/remotes/docker/pusher.go:120 +0xb50
github.com/containerd/containerd/remotes/docker.dockerPusher.Push({0xc0001722d0, {0xc000776619, 0x18}, {0x7f9e18f094c8, 0xc00065e6c0}}, {0x2699d58, 0xc001a42180}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, ...}, ...})
	github.com/containerd/containerd@v1.7.12/remotes/docker/pusher.go:67 +0x10c
github.com/moby/buildkit/util/contentutil.(*pushingIngester).Writer(0xc00047e570, {0x2699d58, 0xc001a42180}, {0xc00041dc70, 0x2, 0x7f9e5fcefa68?})
	github.com/moby/buildkit@v0.13.0-beta1.0.20240126101002-6bd81372ad6f/util/contentutil/pusher.go:76 +0x2e8
github.com/containerd/containerd/content.OpenWriter({0x2699d58, 0xc001a42180}, {0x2676e80, 0xc00047e570}, {0xc00041dc70, 0x2, 0x2})
	github.com/containerd/containerd@v1.7.12/content/helpers.go:115 +0xc3
github.com/containerd/containerd/remotes.Fetch({0x2699d58, 0xc001a42180}, {0x2676e80, 0xc00047e570}, {0x267ba60, 0xc00065e768}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, 0x47}, ...})
	github.com/containerd/containerd@v1.7.12/remotes/handlers.go:117 +0x26e
github.com/moby/buildkit/util/resolver/limited.FetchHandler.FetchHandler.func1({0x2699d90, 0xc000172280}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, 0x47}, 0x60d4b86, {0x0, 0x0, 0x0}, ...})
	github.com/containerd/containerd@v1.7.12/remotes/handlers.go:104 +0x2fa
github.com/moby/buildkit/util/contentutil.CopyChain.New.func5({0x2699d90, 0xc000172280}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, 0x47}, 0x60d4b86, {0x0, 0x0, 0x0}, ...})
	github.com/moby/buildkit@v0.13.0-beta1.0.20240126101002-6bd81372ad6f/util/resolver/retryhandler/retry.go:25 +0xb9
github.com/containerd/containerd/images.HandlerFunc.Handle(0x1000?, {0x2699d90?, 0xc000172280?}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, 0x47}, 0x60d4b86, {0x0, 0x0, ...}, ...})
	github.com/containerd/containerd@v1.7.12/images/handlers.go:59 +0x63
github.com/moby/buildkit/util/contentutil.CopyChain.Handlers.func6({0x2699d90, 0xc000172280}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, 0x47}, 0x60d4b86, {0x0, 0x0, 0x0}, ...})
	github.com/containerd/containerd@v1.7.12/images/handlers.go:69 +0x15e
github.com/containerd/containerd/images.HandlerFunc.Handle(0xc0007e3680?, {0x2699d90?, 0xc000172280?}, {{0xc00190eae0, 0x2b}, {0xc001115ae0, 0x47}, 0x60d4b86, {0x0, 0x0, ...}, ...})
	github.com/containerd/containerd@v1.7.12/images/handlers.go:59 +0x63
github.com/containerd/containerd/images.Dispatch.func1()
	github.com/containerd/containerd@v1.7.12/images/handlers.go:168 +0xd6
golang.org/x/sync/errgroup.(*Group).Go.func1()
	golang.org/x/sync@v0.4.0/errgroup/errgroup.go:75 +0x56
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 1106
	golang.org/x/sync@v0.4.0/errgroup/errgroup.go:72 +0x96

https://github.com/dunglas/frankenphp/actions/runs/7728429011/job/21072578318

@jedevc
Copy link
Collaborator Author

jedevc commented Jan 31, 2024

oh no

That one looks absolutely miserable 😢 I think this is unrelated thankfully, so we should be able to merge this one now, and follow-up.

Why in the world would we be getting a SIGSEGV in OTEL code? 😢 😢 It's also coming from something containerd pusher related: realllllly hope it's not related to containerd/containerd#8379.

@crazy-max
Copy link
Member

I tried it in FrankenPHP's CI and it looks like it triggers another bug:

@dunglas Would you mind opening another issue with this stacktrace please? Thanks.

@crazy-max
Copy link
Member

Why in the world would we be getting a SIGSEGV in OTEL code? 😢 😢 It's also coming from something containerd pusher related: realllllly hope it's not related to containerd/containerd#8379.

Oh right I see the call to the resolver 😳

@dunglas dunglas mentioned this pull request Feb 1, 2024
Closed
3 tasks
@jedevc jedevc merged commit 8968402 into docker:master Feb 1, 2024
@jedevc jedevc deleted the imagetools-resolver-copy-dupe branch February 1, 2024 10:30
@crazy-max crazy-max mentioned this pull request Feb 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crazy-max crazy-max crazy-max approved these changes

+1 more reviewer

@dunglas dunglas dunglas approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"imagetools create" panics when pushing the created image
3 participants
@jedevc @dunglas @crazy-max