Use os.rename() in async_wrapper #4051
Conversation
|
Thanks @Shrews for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review. [This message brought to you by your friendly Ansibull-bot.] |
| @@ -104,7 +108,9 @@ def _run_module(wrapped_cmd, jid, job_path): | |||
| } | |||
| result['ansible_job_id'] = jid | |||
| jobfile.write(json.dumps(result)) | |||
| jobfile.close() | |||
| finally: | |||
There was a problem hiding this comment.
try/except/finally won't work with 2.4
Because the async_status module will read from the same file that the async_wrapper module is writing, it's possible that the file may not be fully synced during a read, causing spurious failures. Use a temp file to do an atomic operation on the file. We can't use atomic_move() here as that doesn't work properly under async. Also, let's not read concurrently from the same file the subprocess is writing to. Instead, capture stdout/stderr via PIPE and write to the file to avoid nasty races.
|
ready_for_review |
|
Thanks @Shrews for this PR. This module is maintained by the Ansible core team, so it can take a while for patches to be reviewed. Thanks for your patience. Core team: please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with 'needs_revision' or merge as appropriate. [This message brought to you by your friendly Ansibull-bot.] |
ISSUE TYPE
COMPONENT NAME
utilities/logic/async_wrapper.py
ANSIBLE VERSION
This should probably be backported to 2.1 as well.
SUMMARY
Because the async_status module will read from the same file that
the async_wrapper module is writing, it's possible that the file
may not be fully synced during a read, causing spurious failures.
Use a temp file to do an atomic operation on the file. We can't
use atomic_move() here as that doesn't work properly under async.
Also, let's not read concurrently from the same file the subprocess
is writing to. Instead, capture stdout/stderr via PIPE and write to
the file to avoid nasty races.