store: correctly remove incomplete layers on load.#2185
store: correctly remove incomplete layers on load.#2185openshift-merge-bot[bot] merged 1 commit intocontainers:mainfrom
Conversation
cgwalters
left a comment
There was a problem hiding this comment.
Can't resist noting this type of bug can't happen in Rust.
In this case, probably by deleting an image (or a single layer): that marks layers as incomplete and proceeds to delete the layer files. If the deletion is aborted (by panic / reboot / SIGKILL) at that time, the layer remains as incomplete and triggers this late deletion. (It would also be necessary to arrange that the layer being deleted is not the last one in I don’t think there is a way to write a reliable test for this (using the stable API); the caller would have to trigger deletion of a layer in one thread, and abort the process in another, hoping that it aborts it during the filesystem deletion. (Alternatively, a hack would be to call |
|
/approve |
In go one should never modify a slice while also iterating over it at
the same time. This causes weird side effects as the underlying array
elements are shifted around without the range loop index knowing.
So if you delete a element the loop will then actually skip the next one
and theoretically access out of bounds on the last element which does
not panic but rather return the default zero type, nil here which then
causes the panic on layer.Flags == nil.
Here is a simple example to show the behavior:
func main() {
slice := []int{1, 2, 3, 4, 5, 6, 7, 8, 9}
for _, num := range slice {
if num == 5 {
slice = slices.DeleteFunc(slice, func(n int) bool {
return n == 5
})
}
fmt.Println(num)
}
}
The loop will not print 6, but then as last number it prints 0 (the
default zero type for an int).
Fixes containers#2184
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, giuseppe, Luap99, mtrmac The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
link back on #24843 (sorry, this log from cockpit web console) When I built from source, there were no errors (I mean with 'make') |
In go one should never modify a slice while also iterating over it at the same time. This causes weird side effects as the underlying array elements are shifted around without the range loop index knowing. So if you delete a element the loop will then actually skip the next one and theoretically access out of bounds on the last element which does not panic but rather return the default zero type, nil here which then causes the panic on layer.Flags == nil.
Here is a simple example to show the behavior:
The loop will not print 6, but then as last number it prints 0 (the default zero type for an int).
Fixes #2184