fix(python): process.terminate() got an unexpected keyword argument 'timeout' #2421
Conversation
RomainMuller
force-pushed
the
rmuller/fix-python-temrinate
branch
from
97ae04d to
a500444
Compare
…timeout' Corrected usage of `process.terminate` to remove unexpected `timeout` argument. Not sure how this did not surface in integration tests (will investigate later). Fixes #2398
RomainMuller
force-pushed
the
rmuller/fix-python-temrinate
branch
from
a500444 to
2d26a85
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| self._process.terminate(timeout=5) | ||
| except subprocess.TimeoutExpired: | ||
| self._process.kill() | ||
| self._process.terminate() |
There was a problem hiding this comment.
Note: terminate does the same work as kill, except on POSIX platforms, where the signal sent to the subprocess is different (SIGTERM for terminate, SIGKILL for kill). The default handling for both of these signals is the same (terminates the process).
SIGKILL cannot be trapped (always kills the process immediately, not giving it a chance to clean up after itself).
SIGTERM can be trapped (and rescued to not terminate the process), and gives the process a modest slice of time to clean up after itself. Since we are not trapping SIGTERM, both signals achieve the same - terminating the node VM, except SIGTERM would allow VM exit handlers to run, which is desirable.
Corrected usage of
process.terminateto remove unexpectedtimeoutargument. Not sure how this did not surface in integration tests (will
investigate later).
Fixes #2398
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.