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

[saschagrunert]

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]

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.