Skip to content

Conversation

@tonistiigi
Copy link
Member

For active builds these records get stored in the memory and marshalled on user request. This leads to possibility that the original record may be updated while the marshalling is happening, leading to a crash.

For active builds these records get stored in the memory
and marshalled on user request. This leads to possibility that
the original record may be updated while the marshalling is
happening, leading to a crash.

Signed-off-by: Tonis Tiigi <[email protected]>
@tonistiigi tonistiigi requested a review from jsternberg January 15, 2025 19:00
@tonistiigi tonistiigi added this to the v0.19.0 milestone Jan 15, 2025
h.mu.Lock()
defer h.mu.Unlock()

e = e.CloneVT()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this happen outside of the lock?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The race is not with this function itself. It is that the record gets saved in the actives and remains there after this function has returned.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't which is why I approved it but it might benefit to perform an action that doesn't require the lock outside of the lock so it doesn't hold the lock as long.

Probably not a hotspot though so doesn't matter much.

@thompson-shaun thompson-shaun merged commit f101ab5 into moby:master Jan 16, 2025
97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants