mirror of
https://github.com/ansible-collections/community.general.git
synced 2025-06-10 18:29:11 -07:00
Make the timeout decorator raise an exception out of the function's scope (#49921)
* Revert "allow caller to deal with timeout (#49449)"
This reverts commit 63279823a7
.
Flawed on many levels
* Adds poor API to a public function
* Papers over the fact that the public function is doing something bad
by catching exceptions it cannot handle in the first place
* Papers over the real cause of the issue which is a bug in the timeout
decorator
* Doesn't reraise properly
* Catches the wrong exception
Fixes #49824
Fixes #49817
* Make the timeout decorator properly raise an exception outside of the function's scope
signal handlers which raise exceptions will never work well because the
exception can be raised anywhere in the called code. This leads to
exception race conditions where the exceptions could end up being
hanlded by unintended pieces of the called code.
The timeout decorator was using just that idiom. It was especially bad
because the decorator syntactically occurs outside of the called code
but because of the signal handler, the exception was being raised inside
of the called code.
This change uses a thread instead of a signal to manage the timeout in
parallel to the execution of the decorated function. Since raising of
the exception happens inside of the decorator, now, instead of inside of
a signal handler, the timeout exception is raised from outside of the
called code as expected which makes reasoning about where exceptions are
to be expected intuitive again.
Fixes #43884
* Add a common case test.
Adding an integration test driven from our unittests. Most of the time
we'll timeout in run_command which is running things in a subprocess.
Create a test for that specific case in case anything funky comes up
between threading and execve.
* Don't use OSError-based TimeoutError as a base class
Unlike most standard exceptions, OSError has a specific parameter list
with specific meanings. Instead follow the example of other stdlib
functions, concurrent.futures and multiprocessing and define a separate
TimeoutException.
* Add comment and docstring to point out that this is not hte Python3 TimeoutError
This commit is contained in:
parent
2b48c0187c
commit
bd072fe83a
3 changed files with 78 additions and 28 deletions
|
@ -2605,7 +2605,7 @@ class AnsibleModule(object):
|
|||
|
||||
def run_command(self, args, check_rc=False, close_fds=True, executable=None, data=None, binary_data=False, path_prefix=None, cwd=None,
|
||||
use_unsafe_shell=False, prompt_regex=None, environ_update=None, umask=None, encoding='utf-8', errors='surrogate_or_strict',
|
||||
expand_user_and_vars=True, pass_fds=None, before_communicate_callback=None, raise_timeouts=False):
|
||||
expand_user_and_vars=True, pass_fds=None, before_communicate_callback=None):
|
||||
'''
|
||||
Execute a command, returns rc, stdout, and stderr.
|
||||
|
||||
|
@ -2655,9 +2655,6 @@ class AnsibleModule(object):
|
|||
after ``Popen`` object will be created
|
||||
but before communicating to the process.
|
||||
(``Popen`` object will be passed to callback as a first argument)
|
||||
:kw raise_timeouts: This is a boolean, which when True, will allow the
|
||||
caller to deal with timeout exceptions. When false we use the previous
|
||||
behaviour of having run_command directly call fail_json when they occur.
|
||||
:returns: A 3-tuple of return code (integer), stdout (native string),
|
||||
and stderr (native string). On python2, stdout and stderr are both
|
||||
byte strings. On python3, stdout and stderr are text strings converted
|
||||
|
@ -2831,12 +2828,6 @@ class AnsibleModule(object):
|
|||
cmd.stderr.close()
|
||||
|
||||
rc = cmd.returncode
|
||||
except TimeoutError as e:
|
||||
self.log("Timeout Executing CMD:%s Timeout :%s" % (self._clean_args(args), to_native(e)))
|
||||
if raise_timeouts:
|
||||
raise e
|
||||
else:
|
||||
self.fail_json(rc=e.errno, msg=to_native(e), cmd=self._clean_args(args))
|
||||
except (OSError, IOError) as e:
|
||||
self.log("Error Executing CMD:%s Exception:%s" % (self._clean_args(args), to_native(e)))
|
||||
self.fail_json(rc=e.errno, msg=to_native(e), cmd=self._clean_args(args))
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue