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

Multiple processes competition on read/write update log #1938

Closed
frostming opened this issue Sep 10, 2020 · 3 comments · Fixed by #1941
Closed

Multiple processes competition on read/write update log #1938

frostming opened this issue Sep 10, 2020 · 3 comments · Fixed by #1941
Labels

Comments

@frostming
Copy link
Contributor

frostming commented Sep 10, 2020

Issue

def read(self):
data, bad_format = None, False
try:
data = json.loads(self.file.read_text())
logging.debug("got {} from %s".format(self.msg), *self.msg_args)
return data
except ValueError:
bad_format = True
except Exception: # noqa
pass
if bad_format:
self.remove()
return None

When the file is opened for writing in another process, the JSON content can't be parsed, then the file will be removed. However, on Windows the removal will fail since the file is occupied by anther process.

Environment

Provide at least:

  • OS: Windows 10

  • pip list of the host python where virtualenv is installed:

    appdirs           1.4.4
    argcomplete       1.11.1
    backcall          0.1.0
    better-exceptions 0.2.2
    certifi           2020.4.5.1
    chardet           3.0.4
    colorama          0.4.3
    decorator         4.4.2
    distlib           0.3.1
    filelock          3.0.12
    idna              2.10
    ipython           7.14.0
    ipython-genutils  0.2.0
    jedi              0.17.0
    naipyext          0.5.2
    parso             0.7.0
    pickleshare       0.7.5
    pip               20.1.1
    pipenv            2020.8.13.dev0 d:\workspace\pipenv
    pipx              0.15.1.3
    prompt-toolkit    3.0.5
    Pygments          2.6.1
    requests          2.24.0
    setuptools        46.4.0
    six               1.14.0
    traitlets         4.3.3
    urllib3           1.25.10
    userpath          1.3.0
    virtualenv        20.0.31
    virtualenv-clone  0.5.4
    wcwidth           0.1.9
    wheel             0.34.2

Output of the virtual environment creation

Make sure to run the creation with -vvv --with-traceback:

Traceback (most recent call last):
  File "C:\Users\runneradmin\.virtualenvs\pipenv-6Kr0DpZ2\lib\site-packages\virtualenv\seed\embed\via_app_data\via_app_data.py", line 94, in _get
    do_periodic_update=self.periodic_update,
  File "C:\Users\runneradmin\.virtualenvs\pipenv-6Kr0DpZ2\lib\site-packages\virtualenv\seed\wheels\acquire.py", line 25, in get_wheel
    wheel = from_bundle(distribution, version, for_py_version, search_dirs, app_data, do_periodic_update)
  File "C:\Users\runneradmin\.virtualenvs\pipenv-6Kr0DpZ2\lib\site-packages\virtualenv\seed\wheels\bundle.py", line 20, in from_bundle
    wheel = periodic_update(distribution, for_py_version, wheel, search_dirs, app_data, do_periodic_update)
  File "C:\Users\runneradmin\.virtualenvs\pipenv-6Kr0DpZ2\lib\site-packages\virtualenv\seed\wheels\periodic_update.py", line 41, in periodic_update
    handle_auto_update(distribution, for_py_version, wheel, search_dirs, app_data)
  File "C:\Users\runneradmin\.virtualenvs\pipenv-6Kr0DpZ2\lib\site-packages\virtualenv\seed\wheels\periodic_update.py", line 62, in handle_auto_update
    u_log = UpdateLog.from_dict(embed_update_log.read())
  File "C:\Users\runneradmin\.virtualenvs\pipenv-6Kr0DpZ2\lib\site-packages\virtualenv\app_data\via_disk_folder.py", line 140, in read
    self.remove()
  File "C:\Users\runneradmin\.virtualenvs\pipenv-6Kr0DpZ2\lib\site-packages\virtualenv\app_data\via_disk_folder.py", line 144, in remove
    self.file.unlink()
  File "C:\hostedtoolcache\windows\Python\3.6.8\x64\lib\pathlib.py", line 1284, in unlink
    self._accessor.unlink(self)
  File "C:\hostedtoolcache\windows\Python\3.6.8\x64\lib\pathlib.py", line 387, in wrapped
    return strfunc(str(pathobj), *args)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\runneradmin\\AppData\\Local\\pypa\\virtualenv\\wheel\\3.8\\embed\\1\\wheel.json'
PermissionError(13, 'The process cannot access the file because it is being used by another process')
RuntimeError: seed failed due to failing to download wheels wheel

To fix the issue, I prefer to change the writing into atomic, that is, before the writing is done, the content should be kept.

If that is an acceptable approach I can send a PR.

@frostming frostming added the bug label Sep 10, 2020
@gaborbernat
Copy link
Contributor

gaborbernat commented Sep 12, 2020

To fix the issue, I prefer to change the writing into atomic, that is, before the writing is done, the content should be kept.

If that is an acceptable approach I can send a PR.

Feel free to put in the PR, and can go from there.

@pfmoore
Copy link
Member

pfmoore commented Sep 14, 2020

Why not simply ignore the error if the file can't be removed? Implementing atomic writes seems to be a fairly complex solution and it's not obvious (to me, at least) why it's better than just failing to remove the file here.

@frostming
Copy link
Contributor Author

frostming commented Sep 15, 2020

@pfmoore This can keep the file readable until the content is changed, instead of returning a None.

I tested my approach and found that it doesn't solve the race issue when run in parallel. On Windows, it may still fail since the file to be overwritten is owned by another process/thread.

So you are right, ignoring the remove failure looks good to me.

@pypa pypa locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants