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

return None instead of 0 in GetStdHandle #1689

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

slozier
Copy link
Contributor

@slozier slozier commented Jun 7, 2023

Resolves an issue reported on gitter by @danabr:

Hi. I have a mysterious issue with subprocess.Popen with stdout=subprocess.PIPE. It works fine from the IronPython console, and also when I run IronPython2 embedded in my application. However, when I use embedded IronPython3, I get: OSError: [WinError 6] The handle is invalid. Here is a stacktrace:

File "myscript.py", line 137, in main
    p = subprocess.Popen(commands, shell=True, stdout=subprocess.PIPE)
  File "lib\subprocess.py", line 833, in __init__
    (p2cread, p2cwrite,
  File "lib\subprocess.py", line 1033, in _get_handles
    p2cread = self._make_inheritable(p2cread)
  File "lib\subprocess.py", line 1080, in _make_inheritable
    h = _winapi.DuplicateHandle(

The issue only occurs when I the stdin argument is not supplied or is set to None. The code in subprocess.py then tries to get the default stdin handle with _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE). That call returns 0 in this case.

The code then goes on to compare the result with None, which seems wrong. From the docs of GetStdHandle:

If an application does not have associated standard handles, such as a service running on an interactive desktop, and has not redirected them, the return value is NULL.

@slozier slozier merged commit 9b71691 into IronLanguages:master Jun 7, 2023
@slozier slozier deleted the getstdhandle branch June 7, 2023 23:35
@@ -15,7 +15,7 @@
from iptest import IronPythonTestCase, is_cli, is_netcoreapp, retryOnFailure, run_test, skipUnlessIronPython

SSL_URL = "www.python.org"
SSL_ISSUER = "CN=GlobalSign Atlas R3 DV TLS CA 2022 Q3, O=GlobalSign nv-sa, C=BE"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change really needed for the _winapi stuff?

Copy link
Contributor

Choose a reason for hiding this comment

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

nm, I see now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just wanted the green checkmark so I snuck it in. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants