Skip to content

bpo-37091: subprocess - Handle uncaught PermissionError in send_signal#13669

Closed
wesinator wants to merge 2 commits into
python:masterfrom
wesinator:patch-1
Closed

bpo-37091: subprocess - Handle uncaught PermissionError in send_signal#13669
wesinator wants to merge 2 commits into
python:masterfrom
wesinator:patch-1

Conversation

@wesinator

@wesinator wesinator commented May 29, 2019 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

Encountered a condition where uncaught PermissionError caused a hang
running a subprocess command with sudo -u

Python 3.7.3
Ubuntu 18.10 Cosmic

Traceback (most recent call last):
  File "/usr/lib/python3.7/subprocess.py", line 474, in run
    stdout, stderr = process.communicate(input, timeout=timeout)
  File "/usr/lib/python3.7/subprocess.py", line 939, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "/usr/lib/python3.7/subprocess.py", line 1707, in _communicate
    self.wait(timeout=self._remaining_time(endtime))
  File "/usr/lib/python3.7/subprocess.py", line 990, in wait
    return self._wait(timeout=timeout)
  File "/usr/lib/python3.7/subprocess.py", line 1616, in _wait
    raise TimeoutExpired(self.args, timeout)
subprocess.TimeoutExpired: Command '['sudo', '-u', 'chrome', '/snap/bin/chromium', '--headless', '--disable-gpu', '--hide-scrollbars', '--ignore-certificate-errors', '--enable-sandbox', '--incognito', '--mute-audio', '--disable-databases', '--enable-strict-powerful-feature-restrictions', '--no-pings', '--no-referrers', '--user-agent="Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; rv:11.0) like Gecko"', '--timeout=30000', '--window-size=1280,1000', '--screenshot=[REDACTED]_screenshot.png', 'https://[REDACTED]']' timed out after 59.9998986274004 seconds

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.7/subprocess.py", line 476, in run
    process.kill()
  File "/usr/lib/python3.7/subprocess.py", line 1756, in kill
    self.send_signal(signal.SIGKILL)
  File "/usr/lib/python3.7/subprocess.py", line 1746, in send_signal
    os.kill(self.pid, sig)
PermissionError: [Errno 1] Operation not permitted

see also xonsh/xonsh#2887 (comment)

https://bugs.python.org/issue37091

@wesinator
Handle uncaught PermissionError in send_signal
f78b2c7 
Encountered a condition where uncaught PermissionError caused a hang

running a subprocess command with `sudo -u`

see also xonsh/xonsh#2887 (comment)
@wesinator wesinator requested a review from gpshead as a code owner May 29, 2019 22:16
gpshead
gpshead requested changes May 29, 2019
View reviewed changes

@gpshead gpshead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just surround the os.kill call with this. Also, please file a related https://bugs.python.org/ issue to track the bug.

@bedevere-bot

bedevere-bot commented May 29, 2019

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@wesinator wesinator changed the title subprocess - Handle uncaught PermissionError in send_signal bpo-37091: subprocess - Handle uncaught PermissionError in send_signal May 29, 2019
@wesinator

wesinator commented May 30, 2019

Copy link
Copy Markdown
Contributor Author

Hmm, this is still hanging with the patch:

Traceback (most recent call last):
  File "/usr/lib/python3.7/subprocess.py", line 474, in run
    stdout, stderr = process.communicate(input, timeout=timeout)
  File "/usr/lib/python3.7/subprocess.py", line 939, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "/usr/lib/python3.7/subprocess.py", line 1707, in _communicate
    self.wait(timeout=self._remaining_time(endtime))
  File "/usr/lib/python3.7/subprocess.py", line 990, in wait
    return self._wait(timeout=timeout)
  File "/usr/lib/python3.7/subprocess.py", line 1616, in _wait
    raise TimeoutExpired(self.args, timeout)

the code has the following except blocks:

except subprocess.TimeoutExpired as e:
        try:
            print(e)
        except PermissionError:
            print(e)
# handle race condition hanging issue
except PermissionError:
    pass

@gpshead

gpshead commented Jun 8, 2019

Copy link
Copy Markdown
Member

This PR would change the behavior of send_signal to hide errors from people. We don't want to do that.

@gpshead gpshead closed this Jun 8, 2019
@wesinator wesinator deleted the patch-1 branch June 8, 2019 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead requested changes

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@wesinator @bedevere-bot @gpshead @the-knights-who-say-ni