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

Lock Finalization Cleanup #11796

Merged
merged 3 commits into from
Dec 20, 2022
Merged

Lock Finalization Cleanup #11796

merged 3 commits into from
Dec 20, 2022

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Dec 19, 2022

What type of PR is this?

Bug Fix

What does this PR do? Why is it needed?

The go runtime makes no guarantees that the finalizer function will be executed after the execution of the last method.
https://pkg.go.dev/runtime#SetFinalizer

However, there is no guarantee that KeepAlive(x) or any other use of x “synchronizes before” f(x), so in general a finalizer should use a mutex or other synchronization mechanism if it needs to access mutable state in x. For example, consider a finalizer that inspects a mutable field in x that is modified from time to time in the main program before x becomes unreachable and the finalizer is invoked.

This would require that in the event we are running our state cleanup, we lock the state when deleting fields to prevent data races and potential panics for this particular case.

Which issues(s) does this PR fix?

N.A

Other notes for review

@nisdas nisdas added the Ready For Review A pull request ready for code review label Dec 19, 2022
@nisdas nisdas requested a review from a team as a code owner December 19, 2022 11:25
@nisdas nisdas merged commit 90d5f60 into develop Dec 20, 2022
@delete-merged-branch delete-merged-branch bot deleted the lockFinalizer branch December 20, 2022 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants