search

google / yapf

A formatter for Python files
Apache License 2.0
13.75k stars 888 forks source link

Fix pickle related race condition (fixes #1164, fixes #1204) #1243

Open hartwork opened 4 days ago

hartwork commented 4 days ago

Fixes #1164 Fixes #1204

I have flaky pre-commit CI caused by Yapf in multiple repositories by now and it was about time to start doing something about it… I'm grateful for the great analysis by both @a-gardner1 and @whlook.

~The pull request sits on top of #1242 for the moment purely for a chance at a green CI: I'm happy to rebase off of it after that one has been merged.~

For a convenient reproducer of the race condition based on earlier work by @whlook please see https://github.com/google/yapf/issues/1204#issuecomment-2391417931 .

If this could be reviewed and merged soon-ish, that would rock the house. Thanks in advance! :pray:

CC @bwendling @whlook @a-gardner1 @kamahen

a-gardner1 commented 4 days ago

Looks good to me. Thanks for picking it up! Do you think a unit test could be added to verify that the reproducer is fixed and does not regress in the future?

hartwork commented 4 days ago

Looks good to me. Thanks for picking it up! Do you think a unit test could be added to verify that the reproducer is fixed and does not regress in the future?

@a-gardner1 that's an interesting idea. I'm happy to add a test based on that reproducer once it's the last thing considered blocking a merge.

hartwork commented 3 days ago

I'm happy to rebase off of it after that one has been merged.

Done.

hartwork commented 1 day ago

@bwendling any chance you could review the fix this coming weak? The bug is quite a bummer and I'm happy to make adjustments where needed. Thank you! :pray:

hartwork commented 1 hour ago

@bwendling many thanks for the review and merge! Is there a chance for a new release v0.40.3 include this pull request to get the fixes to the users? Please let me know if you could use any help on that end :pray: