Skip to content
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.

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

Rootless runs execute storage.GetStore() twice (locking issue) #4591

Closed
saschagrunert opened this issue Nov 28, 2019 · 1 comment · Fixed by #4597
Closed

Rootless runs execute storage.GetStore() twice (locking issue) #4591

saschagrunert opened this issue Nov 28, 2019 · 1 comment · Fixed by #4597
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@saschagrunert
Copy link
Member

The fact that we initialize the store twice in rootless execution mode presumably leads into locking issues with the containers/storage implementation.

For example this works fine in parallel:

> sudo podman push ...
# takes some time
> sudo podman ps
CONTAINER ID  IMAGE                            COMMAND  CREATED         STATUS            PORTS  NAMES

Listing the running containers in parallel to a push works fine in root mode.

Now in rootless mode:

> podman push ...
# takes time again
> podman ps
# blocks until the push finishes

If we put a debug.PrintStack() here:
https://github.com/containers/libpod/blob/aa95726c98385fb75bc70b6de7a86c09ed74abd5/libpod/runtime.go#L732-L736

then we see that we have two stack traces when doing the push:

goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x0, 0xc00054eff8)
        /usr/lib64/go/1.13/src/runtime/debug/stack.go:24 +0x9d
runtime/debug.PrintStack()
        /usr/lib64/go/1.13/src/runtime/debug/stack.go:16 +0x22
github.com/containers/libpod/libpod.(*Runtime).configureStore(0xc0001e2380, 0x0, 0x0)
        libpod/runtime.go:734 +0x37
github.com/containers/libpod/libpod.makeRuntime(0x1cc2a00, 0xc0000480f8, 0xc0001e2380, 0x0, 0x0)
        libpod/runtime.go:294 +0x22cc
github.com/containers/libpod/libpod.newRuntimeFromConfig(0x1cc2a00, 0xc0000480f8, 0x0, 0x0, 0xc000344c38, 0x1, 0x1, 0x1, 0x41d232, 0xc0001c77d8)
        libpod/runtime.go:151 +0x150
github.com/containers/libpod/libpod.NewRuntime(...)
        libpod/runtime.go:120
github.com/containers/libpod/cmd/podman/libpodruntime.getRuntime(0x1cc2a00, 0xc0000480f8, 0xc0001c7af8, 0x1000000, 0x0, 0x0, 0xc00047f450, 0xc00034aa80, 0x14)
        cmd/podman/libpodruntime/runtime.go:183 +0xc8b
github.com/containers/libpod/cmd/podman/libpodruntime.GetRuntime(...)
        cmd/podman/libpodruntime/runtime.go:33
main.setupRootless(0x2ad6f20, 0xc0003efcc0, 0x1, 0x2, 0x0, 0x0)
        cmd/podman/main_local.go:162 +0x14a
main.before(0x2ad6f20, 0xc0003efcc0, 0x1, 0x2, 0x1, 0xc0003449c0)
        cmd/podman/main.go:124 +0xd0
main.glob..func68(0x2ad6f20, 0xc0003efcc0, 0x1, 0x2, 0x0, 0x0)
        cmd/podman/main.go:78 +0x49
github.com/spf13/cobra.(*Command).execute(0x2ad6f20, 0xc00003c120, 0x2, 0x2, 0x2ad6f20, 0xc00003c120)
        vendor/github.com/spf13/cobra/command.go:805 +0x56b
github.com/spf13/cobra.(*Command).ExecuteC(0x2adba20, 0x6, 0x19c8dae, 0x8)
        vendor/github.com/spf13/cobra/command.go:914 +0x2fb
github.com/spf13/cobra.(*Command).Execute(...)
        vendor/github.com/spf13/cobra/command.go:864
main.main()
        cmd/podman/main.go:160 +0xb0

goroutine 1 [running]:
runtime/debug.Stack(0x0, 0x0, 0xc00054f128)
        /usr/lib64/go/1.13/src/runtime/debug/stack.go:24 +0x9d
runtime/debug.PrintStack()
        /usr/lib64/go/1.13/src/runtime/debug/stack.go:16 +0x22
github.com/containers/libpod/libpod.(*Runtime).configureStore(0xc0002c8000, 0x0, 0x0)
        libpod/runtime.go:734 +0x37
github.com/containers/libpod/libpod.makeRuntime(0x1cc2a00, 0xc0000480f8, 0xc0002c8000, 0x0, 0x0)
        libpod/runtime.go:294 +0x22cc
github.com/containers/libpod/libpod.newRuntimeFromConfig(0x1cc2a00, 0xc0000480f8, 0x0, 0x0, 0xc00056e018, 0x1, 0x1, 0x1, 0x8, 0x19c2943)
        libpod/runtime.go:151 +0x150
github.com/containers/libpod/libpod.NewRuntime(...)
        libpod/runtime.go:120
github.com/containers/libpod/cmd/podman/libpodruntime.getRuntime(0x1cc2a00, 0xc0000480f8, 0x2b5ba80, 0x1000000, 0x0, 0x0, 0x6, 0x1, 0xc00054fb28)
        cmd/podman/libpodruntime/runtime.go:183 +0xc8b
github.com/containers/libpod/cmd/podman/libpodruntime.GetRuntime(...)
        cmd/podman/libpodruntime/runtime.go:33
github.com/containers/libpod/pkg/adapter.GetRuntime(0x1cc2a00, 0xc0000480f8, 0x2b5ba80, 0x7, 0xc00054fb80, 0x100)
        pkg/adapter/runtime.go:74 +0x53
main.pushCmd(0x2b5ba80, 0x0, 0x0)
        cmd/podman/push.go:95 +0x17d
main.glob..func59(0x2ad6f20, 0xc0003efcc0, 0x1, 0x2, 0x0, 0x0)
        cmd/podman/push.go:36 +0x86
github.com/spf13/cobra.(*Command).execute(0x2ad6f20, 0xc00003c120, 0x2, 0x2, 0x2ad6f20, 0xc00003c120)
        vendor/github.com/spf13/cobra/command.go:826 +0x460
github.com/spf13/cobra.(*Command).ExecuteC(0x2adba20, 0x6, 0x19c8dae, 0x8)
        vendor/github.com/spf13/cobra/command.go:914 +0x2fb
github.com/spf13/cobra.(*Command).Execute(...)
        vendor/github.com/spf13/cobra/command.go:864
main.main()
        cmd/podman/main.go:160 +0xb0

I'm currently playing around with containers/storage locking enhancements (containers/storage#473, containers/storage#420), where it is not possible for me to bring the enhancement into rootless podman without solving that issue first.

Maybe we should retrieve the store not during runtime initialization, but later right before the command execution... 🤔 Might also be that the issue is somewhere in this logic:
https://github.com/containers/libpod/blob/aa95726c98385fb75bc70b6de7a86c09ed74abd5/libpod/runtime.go#L286-L306

@mheon
Copy link
Member

mheon commented Nov 28, 2019

Is this pulling the image holding the storage locks for the duration of the pull? That's really not optimal if it's true.

The primary rootless use of the second runtime is here - https://github.com/containers/libpod/blob/master/cmd/podman/main_local.go#L162-L177

I don't see anything there that actually needs a store. We have the ability to make a libpod runtime without a store via WithNoStore() - if we can use that for the initial rootless GetRuntime() invocation, we can avoid the storage locks entirely.

saschagrunert added a commit to openSUSE/libpod that referenced this issue Nov 29, 2019
This fixes a double-locking issue of the container storage when running
rootless podman.

Closes containers#4591

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
saschagrunert added a commit to openSUSE/libpod that referenced this issue Nov 29, 2019
This fixes a double-locking issue of the container storage when running
rootless podman.

Closes containers#4591

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants