drivers/quota.Control: lock a mutex when called

[nalind]

The "quotas" map and "next project ID" field don't take well to being created and updated concurrently in multiple threads. Throw a lock around the whole object when any of its exported methods are called. Fixes #1394.

[rhatdan]

LGTM

[mtrmac]

(I appreciate that this fixes #1394 as is, and it’s better to have this PR than to have nothing. It’s just… a whole new can of worms I was not aware of.

Even if the individual disk usage computation was fine vs. concurrent updates/removals, and it would be fine to return such mostly-invalid data, I’m at least worried about things like Driver.dir not being aware of the need to correctly work outside of the usual locks.)

[nalind]

It's not exposed through the Store object. I think this is about library consumers calling into the quota package directly, which is something podman has apparently been doing for a while.

[nalind]

Rebased.

[mtrmac]

The CRI-O backtrace in the original report points at https://github.com/cri-o/cri-o/blob/80228a5d6a4b26e9ed2363ca6a936f5fb604c607/internal/lib/stats/stats_server.go#L189-L197 , which uses a published method of Store: https://github.com/containers/storage/blob/9e425af19496438e8f50f25494d2771166a3c2b5/store.go#L154 .

(The Podman user I can find is https://github.com/containers/podman/blob/b15510694b81929dbc107d354fdb0a83d0bfaa2a/libpod/runtime_volume_common.go#L181-L197 , but that’s a private single-use quota.Control, so I can’t see that that one requires locking.)

[nalind]

Yikes, I thought we'd dropped that somewhere along the way.

[mtrmac]

WDYT about marking Store.GraphDriver deprecated?

That would require giving CRI-O a new Store API to get the container size, and that could lock via readAllLayerStores. And something to expose Driver.Metadata for Podman’s inspect functions.