python / cpython

The Python programming language
https://www.python.org
Other
62.42k stars 29.97k forks source link

pythonw should set `sys.stdout` and `sys.stdin` to valid objects. #122633

Open ChrisBarker-NOAA opened 1 month ago

ChrisBarker-NOAA commented 1 month ago

Bug report

Bug description:

So not exactly a bug -- but I wouldn't call it a feature request either, so I filed it as a bug.

The issue is that pythonw on Windows sets sys.stdout and sys.stdin to None.

This is problematic because while, e.g. print() will not fail, if not do anything useful, other things like warnings and logging, etc., may try to use stdout or stderr, and they will then fail.

See #107792 and #57791

So one might say: "don't do that" -- fair enough, except the user suffering from the problem may not be the one that wrote the code that's calling stderr -- it could be in a library, for instance. Library writers are often not thinking about windowed apps, and shouldn't have to do something special to accommodate them in any case.

See this note in the PyInstaller docs:

https://pyinstaller.org/en/stable/common-issues-and-pitfalls.html?highlight=os.devnull#sys-stdin-sys-stdout-and-sys-stderr-in-noconsole-windowed-applications-windows-only

In which the solution is:

import sys
import os

if sys.stdout is None:
    sys.stdout = open(os.devnull, "w")
if sys.stderr is None:
    sys.stderr = open(os.devnull, "w")

It's not unreasonable for library authors to expect that stdout and stderr to be valid.

So couldn't pythonw add that code by default? users could override it themselves, of course.

NOTE: tested recently on py3.12 -- but this has been an issue for quite some time.

NOTE2: this isn't an issue on the Mac, because its pythonw redirects to the system off-screen console.

CPython versions tested on:

3.12

Operating systems tested on:

Windows

eryksun commented 1 month ago

The current behavior is documented. Maybe some applications or libraries depend on the documented behavior in order to handle the lack of standard I/O.

GadgetSteve commented 1 month ago

Wouldn't it be better if by default pythonw had stdout & stderr as os.devnull unless a flag, e.g. PYW_STD_PIPES_NONE is set? While this would be a change from the current documented behaviour the flag would allow any applications that do depend on this to work and it would save many developers from hitting this as an error. Maybe it would also be a nice option to add for both streams to be redirected to a logger instance with a flag such as PYW_STD_PIPES_LOG this would have saved me on a few occasions from having to re-build windowed apps to CLI so as to see what was going wrong on an end users system.

bwoodsend commented 1 month ago

Wouldn't it be better if by default pythonw had stdout & stderr as os.devnull unless a flag, e.g. PYW_STD_PIPES_NONE is set? While this would be a change from the current documented behaviour the flag would allow any applications that do depend on this to work and it would save many developers from hitting this as an error.

That would still put the burden on every library maintainer ever to be aware that the standard pipes are not guaranteed to be there. Granted, the chance of it breaking would be less but it would still mean that any direct sys.stderr.write() call without an appropriate if sys.stderr is not None guard is technically broken, non-portable code.

GadgetSteve commented 1 month ago

Granted, the chance of it breaking would be less but it would still mean that any direct sys.stderr.write() call without an appropriate if sys.stderr is not None guard is technically broken, non-portable code.

If sys.stdout & sys.stderr are assigned to either os.devnull or to logger objects with INFO & ERROR levels respectively then direct calls to the write methods would not fail. It is worth noting that pythonw is technically inherently non-portable. I understand that on macOS print such writes go to a hidden terminal in windowed mode so neither pipe is None there.

Thinking about it it might be simplest to assign pythonws stdout and stderr to a logger all of the time, with the default being to attach to the logging.NullHandler this could give simpler code.

I haven't looked into the details of implementing this yet but if there is general support I would be willing to give it a go.

bwoodsend commented 1 month ago

Assuming that the os.devull approach doesn't float, shouldn't mypy be expected to catch issues with direct sys.stdxxx access? It looks like this MaybeNone is preventing it from detecting anything. (I don't use typing at all so I don't really know the rules.)

ChrisBarker-NOAA commented 1 month ago

The current behavior is documented. Maybe some applications or libraries depend on the documented behavior in order to handle the lack of standard I/O.

Well, that's always the challenge with software and bugs/misfeatures.

Obligatory xkcd reference:

https://xkcd.com/1172/

But anyway-- this only applies to windowed apps on Windows-- and it's Very unlikely that anyone is using "is None" to mean anything other than "is not a valid object" -- because there are other ways to check if an app is frozen, or a GUI app, or running in windows, etc.

So if a user wants to something other than a devnull ( or equivalent) they are probably doing something else - or should be.

Note: now that I think about it -- we should probably recommend an ETAFTP approach-- catch the Exception when trying to use it, rather than check for None.

bwoodsend commented 1 month ago

I'd be more worried about some less obvious side effect.

To give an example, PyInstaller used to do a workaround akin to sys.stdout = io.StringIO() which fixed sys.stdout.write() but broke subprocess.run() (unless its stdout parameter was set to either PIPE or DEVNULL) because the default behaviour of using the parent process's stdout involved a sys.stdout.fileno() call somewhere deep inside subprocess.

Obviously that particular error shouldn't apply to os.devnull but I'm not confident that there won't be others.

terryjreedy commented 1 month ago

Documented behavior is not a bug. Changing such is usually a feature-request that requires strong justification and a deprecation process.

pythonw.exe is Windows specific and intended mostly for GUI apps and their installers. For instance, CPython Windows installers use pythonw.exe in the IDLE icons and context menu entries it installs. IDLE itself, as an IDE, executes user code in contexts that connect the std streams to its GUI console. (That IDLE's handling of messages from IDLE itself could/should be improved is an IDLE issue, which would not be affected by the change proposed here) What current usage problem is the justification for a non-trivial change in python.exe?

The similar issues with the 3rd-party cross-platform PyInstaller '--noconsole' command line option, which seems to have a somewhat different purpose, is not directly relevant here. Ditto for comments about macOS behavior. Maybe when the option is given, they should set the streams to devnull for users, but that is their call. Perhaps they do not because doing so is not as trivial as suggested here.

bwoodsend commented 1 month ago

Maybe when the option is given, they should set the streams to devnull for users, but that is their call. Perhaps they do not because doing so is not as trivial as suggested here.

We (I'm a PyInstaller maintainer) don't do it because Python doesn't do it. It would be trivial for us to but we're reluctant to deviate from how regular Python behaves, even if it would save us from a lot of invalid reports (write, flush).

eryksun commented 1 month ago

Note that this isn't just about "pythonw.exe", or any other GUI app that embeds the Python DLL on Windows. The process may lack standard I/O for various reasons. The standard handles could be implicitly inherited as NULL or explicitly set to NULL in the process STARTUPINFOW record. Or "python.exe" (the console app) could be spawned without a console session via the creation flag DETACHED_PROCESS, and without standard I/O explicitly set in its STARTUPINFOW record. Regarding the latter, even a non-console app such as "pythonw.exe" can be explicit spawned with valid standard I/O (e.g. pipe or nul files) set in its STARTUPINFOW record.

On Unix systems, it's trivial to spawn a Python process that lacks one or more of the standard I/O files, though it's not normal to do so. For example:

$ python -c 'import os, subprocess; os.close(0); subprocess.run(["python", "-c", "import sys; print(sys.stdin)"])'
None
ChrisBarker-NOAA commented 1 month ago

Documented behavior is not a bug.

No one said it is. I started this issue with that very sentence. Perhaps there's a better category for this issue -- I'm happy to re-categories it (though I can't see how) -- but this doesn't really feel like it rises to "feature request" exactly either.

Changing such is usually a feature-request that requires strong justification and a deprecation process.

Sure -- how to start that process than in an issue with a discussion like this?

Sure, changing long-standing behavior could break folks' code -- and that shouldn't be done lightly. But we also could never make anything better, ever, if we were not willing to make some changes.

So every time a change is suggested we have to balance the potential benefit and the potential damage.

Finally (for the meta-conversation), maybe I'm just being pedantic or unimaginative, but I'd really appreciate it if folks could make the distinction between:

Yes, this change would be better, but it's not worth breaking folks' code over.

and

The current behavior is a good (best?) option.

Either way, nothing changes, but I think it's helpful, particularly if it ever comes up again to be clear about it -- so folks don't think they haven't been heard or understood.

On to the actual topic:

Yes, this is only about pythonw on Windows. But that doesn't mean that pythonw on MacOS (or Linux behavior) is irrelevant to the conversation -- I think it's a goal of Python to be as platform independent as possible, and at this point, folks can make, test, run, and distribute a desktop GUI app on the Mac, and then have the exact same code fail on Windows -- very annoying (this happened to me recently, hence this issue).

I presume the reason for the current pythonw behavior is that there is no default stdout and stderr on Windows for non-console apps. I also imagine that the decision to use None was made when os.devnull didn't exist. I haven't dug into the history to confirm, because it's not relevant anymore. But I'm assuming choosing None over os.devnull wasn't a carefully considered decision -- if it were, it would be great if anyone could recall why that choice was made.

But now, a few key points:

First: sys.stdout and sys.stderr are often used in libraries, rather than directly by a top level app. So:

Setting them to os.devnull would address all of the above.

In practice there are two cases for desktop apps that might use pythonw:

1) The app author doesn't care (or hasn't thought about) where the std streams go -- in which case os.devnull would be a fine option -- and as close as we can get to what happens under Mac or Linux. (unless there IS an equivalent to the Mac's console on Windows -- in which case, let's use that! -- but I don't think there is :-( ).

2) The app author does care about where the std streams go -- and they write code to set them up the way they want -- redirecting to a log file, displaying in a text window, what have you. In that case, they are redirecting them anyway, it makes no difference at all what the default is.

So -- the proposed change would be better for some folks, and make no difference to others.

In short setting the streams to None does no good for anyone.

I hope I've made the case that the proposed change would have been a better way to handle this in the first place -- but it is worth potentially breaking code to make the change now?

As pointed out earlier in the thread the proposed change would break code that DOES care about the std streams, AND uses is None to detect whether to change / set it.

I suggest that that's a very small set of use-cases -- it's very hard to know for sure of course, but I'm having a hard time imagining an app author caring about where the std streams go, but ONLY if they're None if they are set to who knows what else, then they don't care?

One thing I can imagine is a developer that is only testing on Windows -- then they may use the is None approach, because they don't think carefully about it, and may not know that they will not be None under, e.g. MacOS. In which case they have accidentally written platform specific code. But even then, that would only be for an app that will be run either with a console, in which case they don't want to redirect, or without, in which case, they do -- not common?

Even for those that do use this heuristic, it could be a fairly easy to find and fix if need be. I know it's better to not introduce a breaking change at all, but in fact, this isn't all that different than the situation now. My example:

This came up for me with an app of mine that's been working fine for over 20 years.

I just updated it with some small changes, and to use a recent Python version and libraries.

First of all: updating to recent versions required a number of changes -- that shouldn't surprise anyone -- it's the cost of keeping current.

But relevant to the thread, this issue suddenly arose for me, not because of anything I'd changed, but because one of the libraries I'm using introduced a use of sys.stderr that it hadn't had before -- see above, I doubt it dawned on that library writer that they might break someone's code with this change -- but it did, due to this unfortunate default.

(and it was hard for me to detect, because I was testing on a Mac)

The point being -- making this change will introduce some bugs that need to be fixed, but it also will reduce some bugs as well.

The fact is that poorly chosen defaults cause problems -- particularly when authors of the code that uses those defaults (e.g. library authors) is different from the person that actually has to override the defaults (window'd application authors).

@eryksun wrote:

The process may lack standard I/O for various reasons.

I don't understand the implications of this. if one starts up a python app in a process without standard I/O -- will sys.stdout and sys.stderr always be set to None? or might they be set to something that will then error out when you try to use it?

Are you suggesting that they could be set to os.devnull in this case as well? I like that idea.

In short -- how does this pertain to this proposal?

... PyInstaller [doesn't] do it because Python doesn't do it.

Hmm -- unless someone does come up with a reason, "Python doesn't do it" because of legacy, and the desire not to make a breaking change. Perhaps those some motivations don't apply to PyInstaller. In which case, I suppose it might make sense for PyInstaller to do it -- the breaking change would be in PyInstaller, and only be introduced when the user built a bundle out of an already working app that used the is None check pretty rare, and clear that it's a PyInstaller issue.

eryksun commented 1 month ago

I don't understand the implications of this. if one starts up a python app in a process without standard I/O -- will sys.stdout and sys.stderr always be set to None?

If any of fileno(stdin) (e.g. fd 0), fileno(stdout) (e.g. fd 1), or fileno(stderr) (e.g. fd 2) isn't valid, then the corresponding sys.std* file is set to None. This applies to all of the cases that I discussed, as well as the Unix (Linux) example that I demonstrated. I also clarified that "pythonw.exe" can be executed with valid standard I/O file handles set in its STARTUPINFOW record, and thus this issue doesn't apply unconditionally to "pythonw.exe".

The relevant code is in create_stdio() in "Python/pylifecycle.c", which returns Py_RETURN_NONE (i.e. None) if !_Py_IsValidFD(fd).

https://github.com/python/cpython/blob/151934a324789c58cca9c7bbd6753d735454df5a/Python/pylifecycle.c#L2569-L2584

ChrisBarker-NOAA commented 1 month ago

Thanks Eryk, that all makes sense.

In most cases, having invalid (or non-existent) stdout and stderr is unusual, and in the hands of the person running the application -- if they are trying to run in an invalid environment, then it won't work, and the error they get should be clear. So all good. And folks writing the code can expect the std streams will be there.

I do think it's odd that Windows doesn't have a default for these -- but I guess the expectation (and usual practice) is that the application will set things up the way it needs it. A bit of a throw back to the early days of development, when people wrote apps more from scratch ... In practice, I imagine folks use application frameworks [*] that provide these for them.

In the case at hand, pythonw IS the application framework -- that's its intent, to provide a python interpreter set up to run GUI apps. It's the same on the Mac -- pythonw is customized to run GUI apps. On Linux, I think there's absolutely no difference, yes?

Which is why I think it should, by default be set up to have valid io streams. Maybe not useful ones, but at least not invalid -- hence os.devnull.

@GadgetSteve: I kinda like the logger approach, but it feels a bit heavyweight to me -- in effect, it's the same as os.devnull by default, but with more overhead. And if an app developer wants to set up logging themselves, they'd have to do al the same work anyway, yes?

Also -- in the spirit of platform independence, it's better not to set up a logger by default unless that was the case on all platforms.

[*] -- Hmm, maybe then, the GUI application frameworks should set these up -- I use wxPython, I may look and see what we could add there.

OK - I've reminded myself -- wx.App has a "redirect" option, which when set to True, redirects stdout/err to a popup window. However, when False, it doesn't do anything -- but perhaps it could set them to os.devnull.

But I still think it's better for pythonw to do it, rather than each GUI framework doing it for you.

zooba commented 1 month ago

I'm not totally against initialising stdout et al. to valid IO objects even when there's no underlying stream, but we also need a way to detect that it isn't "real" so that apps can replace it with their own, and a migration path to help library developers learn about it.[^1] Eryk has more that adequately demonstrated that it affects all variants of CPython, so simply detecting pythonw.exe isn't sufficient to replace the correct is None checks.

I'm -1 on our default being anything other than a "drop everything" no-op implementation. If there's no stream connected, there's nothing sensible we can do with the output, so it's still up to the app to detect that scenario and replace the IO objects. But at least it won't error out. (I'd kind of like if we had a nice native initialization API to replace sys.std* with callback functions though, that'd make embedding much nicer.)

[^1]: And help users learn about the environment variables they'll need to restore the old behaviour.

GadgetSteve commented 1 month ago

I 100% agree that the default behaviour should be a No-Op with no errors.

I am wondering if it might be possible to have stdout & stderr connected to a null stream that also gives True for is None - of course this would almost certainly require adding a special case to id() - alternatively we could possibly in cases where either is None it might be possible to extend None to have the write* methods.

Either of these should allow all existing code with the correct checks to carry on as before while resolving the issue.

Documenting this in the place(s) that it noticed by library developers is probably more of a challenge of course which is why I quite like the above ideas. Maybe trying to get a check for the if std* is None added to tools such as pylint, ruff, etc., could be a way to go?

bwoodsend commented 1 month ago

Maybe trying to get a check for the if std* is None added to tools such as pylint, ruff, etc., could be a way to go?

I halfheartedly proposed https://github.com/python/typeshed/pull/12485 but it looks like it'll hit a lot of (legitimately not pythonw-safe) projects so I'd be surprised if it gets accepted.

GadgetSteve commented 1 month ago

Just to clarify I wasn't thinking of asking for a warning if the current check was missing but rather a "you don't necessarily need this anymore after python 3.x" message if the check was present.

Sent from Outlook for Androidhttps://aka.ms/AAb9ysg


From: Brénainn Woodsend @.> Sent: Monday, August 5, 2024 10:11:56 PM To: python/cpython @.> Cc: Steve (Gadget) Barnes @.>; Mention @.> Subject: Re: [python/cpython] pythonw should set sys.stdout and sys.stdin to valid objects. (Issue #122633)

Maybe trying to get a check for the if std* is None added to tools such as pylint, ruff, etc., could be a way to go?

I halfheartedly proposed python/typeshed#12485https://github.com/python/typeshed/pull/12485 but it looks like it'll hit a lot of (legitimately not pythonw-safe) projects so I'd be surprised if it gets accepted.

— Reply to this email directly, view it on GitHubhttps://github.com/python/cpython/issues/122633#issuecomment-2269926843, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABKVUWUWACSQWIWNIAHVX4DZP7TBXAVCNFSM6AAAAABL5MDQXOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRZHEZDMOBUGM. You are receiving this because you were mentioned.Message ID: @.***>

ChrisBarker-NOAA commented 1 month ago

I halfheartedly proposed https://github.com/python/typeshed/pull/12485

Hmm -- I had to look up what MaybeNone means -- and found this:

"We use MaybeNone in cases where something could possibly be None but we don't want to force all users to check for it."

So yeah, I agree that if they could be None, and you want static type checking, then you code should fail the check if you don't handle None.

I suspect that choice was made because they were thinking that they would only be None if the "host" system were misconfigured -- and they very much don't want every use of the stdio. streams to be wrapped with a None check.

Which is the case, except for pythonw, in which case the None is being set by cPython code. Which is why I think it's a misfeature.

zooba commented 1 month ago

that also gives True for is None - of course this would almost certainly require adding a special case to id()

Worse, it requires a special case for is (id() is not the definition of how is works, it's just a visible hint that is often used to explain what is is doing). So that's not going to happen.

Which is the case, except for pythonw, in which case the None is being set by cPython code. Which is why I think it's a misfeature

You mean, except for cases where you don't have standard IO streams. It's not at all unique to pythonw, and even if pythonw were to connect fake streams, fully correct libraries would still have to check (or better yet, avoid the standard streams and let the app specify). You can be as frustrated as you like about the particular case that triggered the issue, but now that we're discussing ecosystem-wide changes to fix it, let's stick to the root cause.

And while making all the people who do type checking aware that they need to double-check these attributes (once, at startup, which AFAIK no type checkers can handle right now) would certainly go a long way towards making libraries less likely to assume them, I don't think it's the right way to approach it. But unfortunately, we can't exactly print a warning either.

Possibly what we need is extra (platform-specific) handling in the final exception handler, so that an unhandled error gets displayed in some useful way even when sys.stderr isn't available. That could also include a message explaining that sys.stderr wasn't available, which ought to help the dev figure out to assign their own.

ChrisBarker-NOAA commented 1 month ago

while making all the people who do type checking aware that they need to double-check these attributes (once, at startup, which AFAIK no type checkers can handle right now) would certainly go a long way towards making libraries less likely to assume them, I don't think it's the right way to approach it.

Nor do I -- do we really think that every use case of stdout and stderr should be wrapped with the possibility of them not being valid? I think the assumption, by library authors, that they are valid is a reasonable one.

Yes, if we can come up with a way to wrap the invalid streams with a smarter object, so that library authors don't need to do anything special, great. But until then, maybe we can make a much simpler change that will help some folks.

You mean, except for cases where you don't have standard IO streams. It's not at all unique to pythonw, and even if pythonw were to connect fake streams, fully correct libraries would still have to check (or better yet, avoid the standard streams and let the app specify). You can be as frustrated as you like about the particular case that triggered the issue, but now that we're discussing ecosystem-wide changes to fix it, let's stick to the root cause.

If you can solve the root cause, great.

But pythonw is unique -- it's not just one case where this happens to come up:

eryksun commented 1 month ago

pythonw is specifically there to provide an interpreter that runs as a GUI app on Windows.

In that case, we know that the stdio streams will always be NULL.

The standard I/O handle values of a process that's spawned for an executable that is not flagged as a console application, such as "pythonw.exe", default to NULL unless they are explicitly set to inheritable handles in the STARTUPINFOW record that initializes the process parameters. It's trivial to implement the latter in Python via subprocess, and it's also possible in PowerShell via .NET System.Diagnostics.Process. On the other hand, it's not possible with the classic CLI shell, CMD, nor with the desktop shell, Explorer. (CMD overrides standard I/O by temporarily changing its own standard handles before spawning the child. This only works if the child is a console app that inherits the parent's console session. Otherwise the system doesn't implicitly inherit standard I/O from the parent.)

The standard I/O handle values of a process that's spawned for an executable that is flagged as a console application, such as "python.exe", may be explicitly set to NULL in the STARTUPINFOW record that initializes the process parameters. Even without explicitly setting the values to NULL, one or more of the standard handle values may be implicitly NULL -- as either inherited or duplicated from the parent process (assuming the child is in the same console session as the parent, i.e. that the spawn doesn't specify either creation flag CREATE_NEW_CONSOLE or CREATE_NO_WINDOW), or if the spawn specifies the creation flag DETACHED_PROCESS (i.e. force creating a non-console process).

At process startup, if a standard I/O handle is NULL, INVALID_HANDLE_VALUE or isn't a valid handle for a character, pipe, or disk file, then the C runtime library initializes the corresponding standard stream (i.e. stdin, stdout, or stderr) using the invalid file-descriptor value _NO_CONSOLE_FILENO (-2). _Py_IsValidFD() immediately returns 0 for a negative fd value.

The C runtime startup behavior is implemented by the function initialize_stdio_handles_nolock() in "lowio/ioinit.cpp", as follows:

        // This handle has not yet been initialized, so  let's see if we can get
        // the handle from the OS:
        intptr_t const os_handle = reinterpret_cast<intptr_t>(GetStdHandle(get_std_handle_id(fh)));

        bool const is_valid_handle = 
            os_handle != reinterpret_cast<intptr_t>(INVALID_HANDLE_VALUE) &&
            os_handle != reinterpret_cast<intptr_t>(nullptr);

        DWORD const handle_type = is_valid_handle
            ? GetFileType(reinterpret_cast<HANDLE>(os_handle))
            : FILE_TYPE_UNKNOWN;

        if (handle_type != FILE_TYPE_UNKNOWN)
        {
            // The file type is known, so we obtained a valid handle from the
            // OS.  Finish initializing the lowio file object for this handle,
            // including the flag specifying whether this is a character device
            // or a pipe:
            pio->osfhnd = os_handle;

            if ((handle_type & 0xff) == FILE_TYPE_CHAR)
                pio->osfile |= FDEV;

            else if ((handle_type & 0xff) == FILE_TYPE_PIPE)
                pio->osfile |= FPIPE;
        }
        else
        {
            // We were unable to get the handles from the OS.  For stdin, stdout,
            // and stderr, if there is no valid OS handle, treat the CRT handle
            // as being open in text mode on a device with _NO_CONSOLE_FILENO
            // underlying it.  We use this value instead of INVALID_HANDLE_VALUE
            // to distinguish between a failure in opening a file and a program
            // run without a console:
            pio->osfile |= FDEV;
            pio->osfhnd = _NO_CONSOLE_FILENO;

            // Also update the corresponding stdio stream, unless stdio was
            // already terminated:
            if (__piob)
                __piob[fh]->_file = _NO_CONSOLE_FILENO;
        }
bwoodsend commented 1 month ago

Surely the way to handle the discrepancies in the handles are none <==> pythonw mode association is to redefine this as if the handles are invalid then set them to devnull? i.e. we just adjust the bottom of the snippet referenced in https://github.com/python/cpython/issues/122633#issuecomment-2267418063 to return devnull instead of Py_RETURN_NONE if !_Py_IsValidFD(fd)? Then we can truly say that sys.stdout is None is no longer a scenario that developers need to worry about.

vstinner commented 1 month ago

sys.stdin, sys.stdout and sys.stderr are also set to None on Unix if their file descriptor is closed, which is a common behavior when running a "daemon" process/service.

Example:

$ ./python
>>> # close stdin
>>> import os; os.close(0)
>>> import subprocess, sys
>>> proc=subprocess.run([sys.executable, '-c', 'import sys; print(sys.stdin)'])
None

I don't think that changing this behavior is a good idea. Too much code rely on the current assumption that standard streams are set to None if their file descriptor is set to None.

zooba commented 1 month ago

We might (with a deprecation period) be able to substitute None for a singleton instance of a (new) io.DisconnectedStream. This can be fairly easily tested directly, and/or we could define its __bool__ to return False so that if not sys.stdin: checks work for all versions (though is not None would not), and it can swallow any write or read calls so that programs silently misbehave instead of silently crashing (hmm... though print() already silently ignores a missing stream, so it's probably not going to misbehave that badly).

But I'm not sure it solves the original problem, which seems to swing somewhere between "there's no output when there's no output streams" and "things crash when there's no output streams". Fixing the second is easier, though it likely defers a problem to a later point, while the first is more valuable if we can do something useful there. We can't make an absent stdin do anything better than returning EOF, so it's just stdout and stderr that would benefit from being able to show the output.

Maybe the most effective way to handle this would be to recommend/contribute to the GUI frameworks something that checks for None and replaces it with their own handler? That works up-and-down level, and has the least compatibility impact by far.

ChrisBarker-NOAA commented 1 month ago

I don't think that changing this behavior is a good idea. Too much code rely on the current assumption that standard streams are set to None if their file descriptor is set to None.

well, there's also too much code that assumes that the standard streams are always valid :(

But I'm only suggesting changing it for pythonw -- and I'm suggesting that there is very little code that, when run under pythonw, assumes that the None means anything other than "Not valid, might as well be os.devnull".

My assessment may be wrong, of course, and there is surely at least some code that is using is None and doing something more meaningful -- and any breaking change is a breaking change, but I still think it's a net benefit -- see my earlier posts for more details on my logic.

@eryksun: thanks for the detailed explanation -- clearly this is a pretty complex issue [*], but I don't think that changes my logic:

1) For a GUI app under Windows, the default, most common behavior, is to have the streams not set. Naive users will be doing that.

2) For most other environments, the most common behavior is to the have the streams set.

3) In any case, when the non-default behavior is in play, it's in the hands of the end-user running the application, not the application developer.

I think it's best that Python well supports the default, most likely use cases well.

Again -- one could fully agree with me about this, and still think it's not enough gain for a breaking change -- fair enough, that's always a tough call.

[*] I do note that C has the _NO_CONSOLE_FILENO sentinel -- too bad we don't have a Python equivalent -- the challenge with None is that can mean too many things :-(

zooba commented 1 month ago
  1. For a GUI app under Windows, the default, most common behavior, is to have the streams not set. Naive users will be doing that.

The question here though is what are they doing instead? This scenario is the most likely to have another framework in play that could check and replace stdout/stderr on startup. Why aren't they?

ChrisBarker-NOAA commented 1 month ago

Maybe the most effective way to handle this would be to recommend/contribute to the GUI frameworks something that checks for None and replaces it with their own handler? That works up-and-down level, and has the least compatibility impact by far.

Sure -- wxPython has that as an option, and I've jsut started an issue suggesting that the default be changed to os.devnull.

But my point is that pythonw IS a nano GUI framework, and it is the "framework" on which the other frameworks build on -- so it should have the best-we-can-have default behavior.

So here's a non-breaking change idea:

Add a start-up flag for Python: SET_INVALID_IO_STREAMS_TO_DEVNULL:

False by default, if True, then invalid IO Streams will be set to os/devnull.

If this is there:

Potentially, after a deprecation (is that the right word?) period , the default could be changes to True -- at least for pythonw. Or not.

And yes -- this is what @GadgetSteve suggested right up front :-)

ChrisBarker-NOAA commented 1 month ago

The question here though is what are they doing instead? This scenario is the most likely to have another framework in play that could check and replace stdout/stderr on startup. Why aren't they?

They are -- certainly in most cases, I'm having trouble imagining an app designed to be run with a point-and-click-on-the-gui that doesn't either: 1) want to use a default console or 2) use a GUI framework.

However, the GUI frameworks may well provide a way to handle this, but at least one (pythonw) doesn't do it by default.

And this comes up in the PyInstaller list. enough that I think it's common enough problem (given that the pool of GUI app developers is small as is).

Anyone know what tkInter does?

But my point is that python itself (at least pythonw) should do the right thing, rather than expecting all the downstream libs (OK -- it's a pretty small list, but ...) to handle it.

zooba commented 1 month ago

But my point is that pythonw IS a nano GUI framework

Okay, except it's not :) It's an executable with the system flag that says "don't create or attach a console" set, since there's no feasible way for a regular user to do that themselves. Code-wise it is literally identical to python.exe.

I'm not totally opposed to a PYTHON_* environment variable to force absent streams to a no-op object rather than None. But I'm not sure it's worth it. And I am opposed to a pythonw-specific environment variable, or a Windows-specific environment variable - if we can't use it everywhere, we should find another approach.

ChrisBarker-NOAA commented 1 month ago

And I am opposed to a pythonw-specific environment variable, or a Windows-specific environment variable

me too -- I didn't mean to suggest that.

if it's a flag or environment variable, it should be usable everywhere.

bwoodsend commented 1 month ago

I don't think that changing this behavior is a good idea. Too much code rely on the current assumption that standard streams are set to None if their file descriptor is set to None.

What exactly are these use cases where a piece of code would do something differently based on a sys.stdout is None check? The only case I can think of is something like...

if sys.stdout is not None:
    print("some output")
else:
    # The user can't see what we print. Better display it some other way...
    something_involving_log_files_or_pop_up_dialogs("some output")

... but that doesn't even work on Linux or macOS since a GUI application launched by the desktop still has valid <_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'> streams.

A search for if sys.stdout is None in public reposirories gives a few hundred hits and every single relevant one of them is either setting sys.stdout to devnull or no-op-ing some console output function (and note that about half of them have comments indicating that this was an unpleasant surprise, and usually a reported bug). If there are other use cases then they are extremely niche.

zooba commented 1 month ago

The "something different" is only really going to be deciding to set it to something else. If it's already set, that indicates that the caller has overridden it in some way, and if not then it's up to the app to pick a reasonable default (such as devnull if it doesn't matter, or some GUI element or log file if it does).

If someone's log files disappear because we change this, they won't be very happy, and it will be our fault.

bwoodsend commented 1 month ago

But why would someone selectively attach sys.stdout to a log file based on a conditional that so inaccurately reflects whether the current sys.stdout is going anywhere readable?

ChrisBarker-NOAA commented 1 month ago

I hate to argue against my own point, but:

someone would selectively attach sys.stdout to a log file based on a conditional that so inaccurately reflects whether the current sys.stdout is going anywhere readable because that's the only obvious way to do it with the current code base.

None means "invalid stream" -- and it's pretty standard practice in Python to use that sentinel to switch on:

If there's a valid stream, the user set it up the way they want, if not, I'd better set it up to a log file.

Which is why I'm only advocating for changing it in pythonw -- in that case, it's far less likely that application writers are doing anything with "is None" than making they error go away.

All the being said -- I suspect it FAR more common for library writers to assume they will be valid, and top-level app writers to set up a redirect to logfile or whatever in a more robust way.

zooba commented 1 month ago

Being able to argue against your own point is a very good thing :) You still get to dismiss the argument.

And you're spot on. The conditional is literally "is stdout currently valid; if not, I'll make it valid", so there's nothing inaccurate about it.

Inaccurate conditionals would be "am I running under pythonw.exe? If so, sys.stdout must be invalid" or the inverse: "if sys.stdout None? If so, I must be running under pythonw.exe". So as long as we're not encouraging that line of thinking, we won't be making things worse.

I suspect it FAR more common for library writers to assume they will be valid

Yeah, and this is the core point. The choice we have to make is whether to make this assumption more likely, more obvious, or always correct.

I'm of the opinion that "more obvious" is the only option, because part of the assumption is that anything written to stdout will be seen by the user, which we can't achieve consistently when there's no console available.

bwoodsend commented 1 month ago

Hmm, fair enough. Although I still think we'd be fixing way more problems than we'd create by making this change. Like I said before, the github search shows many many cases of people being burnt by the current model and not a single case that would lose its log file or be otherwise negatively affected by this change.

zooba commented 1 month ago

GitHub searches are a biased sample, so they only get weighed carefully (though I didn't have to go far to find one that would break). What's more important is what we've told people to do in the past: https://github.com/python/cpython/commit/58cb1b8b0ef200b7f7d4aba82642a63ce7532545

After a long discussion about the problem with Windows GUI apps Guido decided that sys.stdin, stdout and stderr should be None when the C runtime library returns invalid file descriptors for the standard streams.

I haven't tracked down the original discussion from 17 years ago, but the decision is clear enough.

So rather than just looking at code that people have published publicly in, really, the last few years, we need to weigh that for almost two decades, the behaviour has been for these to be None.

So what we need to do is figure out how to let people know that this is changing, bearing in mind that many apps are closed source, many are going to skip versions, and most already work today and shouldn't stop working just to make it so other peoples' new code appears to work first time. The challenge is the transition plan, not coming up with strong enough rationale.

terryjreedy commented 3 weeks ago

In macOS Terminal: pythonw opens interactive 2.7 in Terminal, the same as python. Perhaps it symlinks to python. I am surprised that this works, and puzzled as to why. Does not matter. Neither pythonw3 nor python3w work (command not found), whereas python3 does. From a user perspective, pythonw.exe seems absent for python3. I looked up the file hierarchy from idlelib and found Python, python3, and python3.13 executables, but no 'w' versions.

ChrisBarker-NOAA commented 3 weeks ago

The difference between a command line app and a GUI app on the Mac is that a GUI app must be run from "within an application bundle".

My information is old, but back in the day, the "Framework Build" for Mac (the one distributed by python.org) had a hack to the python executable that made it appear to be inside an application bundle. It was decided that there was no point in having a separate python executable, as there was essentially no cost to the redirection.

So yes, pythonw and python are the same thing on a Framework build on the Mac.

But they are BOTH "gui apps" as opposed to both being "console apps" (which I suppose is the case on Linux, but for the opposite reason)

The core problem here is that Windows doesn't have a default console for GUI apps -- the Mac does, so on the Mac, there's nothing special to be done for the std streams for GUI apps.

@ned-deily: Is this all the same as it once was?

NOTE: the python builds for the Mac on conda-forge DO have a separate pythonw executable -- the "regular" Python is a straight *nix build, and won't work with GUI apps -- there is a separate python.app package that you need to get an executable you can run a GUI app with.

ned-deily commented 3 weeks ago

Is this all the same as it once was?

Sorry, I haven't been following this issue closely and I am not sure how relevant my reply will be because I'm not intimately familiar with the differences between python and pythonw behavior on Windows. And it sounds like there isn't a perfect analogy to a Windows console on other platforms.

AFAIK, it is true that, for python3, on non-Windows platforms built using the cpython configure and Makefile, including on macOS, we don't install any python*w executable links. The main reason for this is that, on those platforms, you normally need to be running in a platform-provided terminal window of some sort, typically but not always under a Unix shell, and typically the standard files (stdin, stdout, and stderr) are by default connected to the terminal window by the terminal emulator, in which case, python doesn't need to do anything special to interact with the user, just like any other system program run in the terminal window. (Historical note: I believe that Python running on the original classic MacOS - not macOS/OSX/Mac OS X - 25+ years ago did have need for a pythonw as the user environment there was more similar to Windows environments of the era - but that is all before my time.)

There is a separate issue of what happens when that program uses a GUI framework to create its own windows, like using Tk in a python tkinter application, including IDLE. Typically, on non-Windows platforms, IDLE can be launched from a terminal window (using with one of the idle3 commands or with a python3 -m idlelib commands) in which case the python interpreter running IDLE will normally start with stderr, stdout, and stdin connected to the initial terminal window. IDLE will redirect stdin and stdout within the Tk windows it creates but it doesn't do anything about stderr (IIRC) in the python interpreter process itself, thus, some error messages will show up in the initial terminal window.

Some Python distributions, including the python.org macOS installer, provide an additional way to launch IDLE; for macOS, there may be an IDLE.app macOS application with a clickable icon. When IDLE is launched this way, there is no terminal or Unix shell involved and thus the python interpreter's process does not have a usable stdin or stdout and stderr is directed by the OS to the system log (I believe). So some relevant messages may not be obvious to the user (see related issue #57791).

Also, on macOS, I believe that whether or not python is built as a framework build, which does create a hidden Python.app bundle to make things easier for GUI frameworks and other macOS GUI-related system calls, isn't really relevant to this discussion. stdin, stdout, and stderr should behave as described above with either a standard unix build or a framework build.

Hope that helps!

ChrisBarker-NOAA commented 3 weeks ago

Thanks @ned-deily!

Am I correct that the "python" executable in the current python.org build still have the hack that lets it run as a GUI app? i.e. it's "inside an app bundle"?

ned-deily commented 3 weeks ago

You are correct but, as I noted in my edited comment, I don't think that is relevant to this issue; it should have no impact on stdin, stdout, and stderr usage and behavior.

terryjreedy commented 3 weeks ago

Thanks Ned. This means that (paraphrased) "IDLE cannot import tkinter. Bye" should go to stderr, not stdout. I should check.