Skip to content

[3.9] bpo-42398: Fix "make regen-all" race condition (GH-23362) #23367

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

Merged
merged 1 commit into from
Nov 18, 2020
Merged

[3.9] bpo-42398: Fix "make regen-all" race condition (GH-23362) #23367

merged 1 commit into from
Nov 18, 2020

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Nov 18, 2020

Fix a race condition in "make regen-all" when make -jN option is used
to run jobs in parallel. The clinic.py script now only use atomic
write to write files. Moveover, generated files are now left
unchanged if the content does not change, to not change the file
modification time.

The "make regen-all" command runs "make clinic" and "make
regen-importlib" targets:

  • "make regen-importlib" builds object files (ex: Modules/_weakref.o)
    from source files (ex: Modules/_weakref.c) and clinic files (ex:
    Modules/clinic/_weakref.c.h)
  • "make clinic" always rewrites all clinic files
    (ex: Modules/clinic/_weakref.c.h)

Since there is no dependency between "clinic" and "regen-importlib"
Makefile targets, these two targets can be run in parallel. Moreover,
half of clinic.py file writes are not atomic and so there is a race
condition when "make regen-all" runs jobs in parallel using make -jN
option (which can be passed in MAKEFLAGS environment variable).

Fix clinic.py to make all file writes atomic:

  • Add write_file() function to ensure that all file writes are
    atomic: write into a temporary file and then use os.replace().
  • Moreover, write_file() doesn't recreate or modify the file if the
    content does not change to avoid modifying the file modification
    file.
  • Update test_clinic to verify these assertions with a functional
    test.
  • Remove Clinic.force attribute which was no longer used, whereas
    Clinic.verify remains useful.

(cherry picked from commit 8fba952)

https://bugs.python.org/issue42398

Fix a race condition in "make regen-all" when make -jN option is used
to run jobs in parallel. The clinic.py script now only use atomic
write to write files. Moveover, generated files are now left
unchanged if the content does not change, to not change the file
modification time.

The "make regen-all" command runs "make clinic" and "make
regen-importlib" targets:

* "make regen-importlib" builds object files (ex: Modules/_weakref.o)
  from source files (ex: Modules/_weakref.c) and clinic files (ex:
  Modules/clinic/_weakref.c.h)
* "make clinic" always rewrites all clinic files
  (ex: Modules/clinic/_weakref.c.h)

Since there is no dependency between "clinic" and "regen-importlib"
Makefile targets, these two targets can be run in parallel. Moreover,
half of clinic.py file writes are not atomic and so there is a race
condition when "make regen-all" runs jobs in parallel using make -jN
option (which can be passed in MAKEFLAGS environment variable).

Fix clinic.py to make all file writes atomic:

* Add write_file() function to ensure that all file writes are
  atomic: write into a temporary file and then use os.replace().
* Moreover, write_file() doesn't recreate or modify the file if the
  content does not change to avoid modifying the file modification
  file.
* Update test_clinic to verify these assertions with a functional
  test.
* Remove Clinic.force attribute which was no longer used, whereas
  Clinic.verify remains useful.

(cherry picked from commit 8fba952)
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@vstinner vstinner deleted the clinic_update39 branch November 18, 2020 16:11
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 18, 2020
…onGH-23367)

Fix a race condition in "make regen-all" when make -jN option is used
to run jobs in parallel. The clinic.py script now only use atomic
write to write files. Moveover, generated files are now left
unchanged if the content does not change, to not change the file
modification time.

The "make regen-all" command runs "make clinic" and "make
regen-importlib" targets:

* "make regen-importlib" builds object files (ex: Modules/_weakref.o)
  from source files (ex: Modules/_weakref.c) and clinic files (ex:
  Modules/clinic/_weakref.c.h)
* "make clinic" always rewrites all clinic files
  (ex: Modules/clinic/_weakref.c.h)

Since there is no dependency between "clinic" and "regen-importlib"
Makefile targets, these two targets can be run in parallel. Moreover,
half of clinic.py file writes are not atomic and so there is a race
condition when "make regen-all" runs jobs in parallel using make -jN
option (which can be passed in MAKEFLAGS environment variable).

Fix clinic.py to make all file writes atomic:

* Add write_file() function to ensure that all file writes are
  atomic: write into a temporary file and then use os.replace().
* Moreover, write_file() doesn't recreate or modify the file if the
  content does not change to avoid modifying the file modification
  file.
* Update test_clinic to verify these assertions with a functional
  test.
* Remove Clinic.force attribute which was no longer used, whereas
  Clinic.verify remains useful.

(cherry picked from commit 8fba952)
(cherry picked from commit c53c3f4)

Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-bot
Copy link

GH-23371 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Nov 18, 2020
Fix a race condition in "make regen-all" when make -jN option is used
to run jobs in parallel. The clinic.py script now only use atomic
write to write files. Moveover, generated files are now left
unchanged if the content does not change, to not change the file
modification time.

The "make regen-all" command runs "make clinic" and "make
regen-importlib" targets:

* "make regen-importlib" builds object files (ex: Modules/_weakref.o)
  from source files (ex: Modules/_weakref.c) and clinic files (ex:
  Modules/clinic/_weakref.c.h)
* "make clinic" always rewrites all clinic files
  (ex: Modules/clinic/_weakref.c.h)

Since there is no dependency between "clinic" and "regen-importlib"
Makefile targets, these two targets can be run in parallel. Moreover,
half of clinic.py file writes are not atomic and so there is a race
condition when "make regen-all" runs jobs in parallel using make -jN
option (which can be passed in MAKEFLAGS environment variable).

Fix clinic.py to make all file writes atomic:

* Add write_file() function to ensure that all file writes are
  atomic: write into a temporary file and then use os.replace().
* Moreover, write_file() doesn't recreate or modify the file if the
  content does not change to avoid modifying the file modification
  file.
* Update test_clinic to verify these assertions with a functional
  test.
* Remove Clinic.force attribute which was no longer used, whereas
  Clinic.verify remains useful.

(cherry picked from commit 8fba952)
(cherry picked from commit c53c3f4)

Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants