Replace stdout and stderr with simple standard printers at Python exit

[vstinner]

BPO	26642
Nosy	@vstinner, @serhiy-storchaka
Files	replace_stdio.patch replace_stdio-2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-07-26.02:58:29.646>
created_at = <Date 2016-03-25.13:13:17.062>
labels = []
title = 'Replace stdout and stderr with simple standard printers at Python exit'
updated_at = <Date 2017-07-26.02:58:29.644>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2017-07-26.02:58:29.644>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-07-26.02:58:29.646>
closer = 'vstinner'
components = []
creation = <Date 2016-03-25.13:13:17.062>
creator = 'vstinner'
dependencies = []
files = ['42290', '42349']
hgrepos = []
issue_num = 26642
keywords = ['patch']
message_count = 8.0
messages = ['262436', '262455', '262456', '262772', '262774', '263255', '263626', '299192']
nosy_count = 2.0
nosy_names = ['vstinner', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'out of date'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue26642'
versions = ['Python 3.6']

[vstinner]

The Python shutdown process is complex and fragile.

It was discussed in the issue bpo-25654 to replace stdout and stderr with simple "standard printers" (implemented in C, don't depend on other Python objects or modules).

Attached patch implements this idea. The patch begins with closing standard stream objects (sys.stdin, sys.stdout, sys.stderr), and then replace stdout and stderr.

[serhiy-storchaka]

There are some questions.

1. Is there a reason only name is closed, not dunder_name? (Josh's question, but I'm interesting too).

2. Is it worth to first replace standard streams with "standard printers", and then close original streams? This allows to log warnings from closing streams.

3. "standard printers" are used at startup and at shutdown. Can we reuse some code? Or save and reuse "standard printers"?

4. Daemons close standard streams and fileno(stdout) can return unrelevant value. Perhaps it is not good idea to recreate closed stdout.

[vstinner]

Serhiy Storchaka added the comment:

1. Is there a reason only name is closed, not dunder_name? (Josh's question, but I'm interesting too).

By default, sys.__stdout__ is sys.stdout. Is it ok to close the same file twice?

2. Is it worth to first replace standard streams with "standard printers", and then close original streams? This allows to log warnings from closing streams.

Yeah, I tried this locally after sending my patch. I think that we
should keep a strong reference and only "decref" after the stream is
replaced.

3. "standard printers" are used at startup and at shutdown. Can we reuse some code? Or save and reuse "standard printers"?

I will check, it's maybe possible to share some code inside pylifecycle.c.

4. Daemons close standard streams and fileno(stdout) can return unrelevant value. Perhaps it is not good idea to recreate closed stdout.

Ah yes, while playing with my patch, I noticed that I replaced stdout
even if sys.stdout was NULL or None.

Closed stdout is a different thing. I guess that we can try to call
stdout.closed() and only replaced it with a standard printer it
closed() returns True (don't do it on error nor if closed() returns
false).

[vstinner]

Patch version 2:

• check if the stream was already "closed" (see below and comment in the patch)
• first replace stream and then close it and DECREF the object
• don't close stdin anymore

1. Is there a reason only name is closed, not dunder_name? (Josh's question, but I'm interesting too).

Fixed.

2. Is it worth to first replace standard streams with "standard printers", and then close original streams? This allows to log warnings from closing streams.

Fixed.

3. "standard printers" are used at startup and at shutdown. Can we reuse some code?

I looked at the code creating standard printer during Python startup: it's just a few lines and it doesn't handle the case when stdout/stderr is already open. I don't think that it's worth to reuse code.

Anyway, with my new patch, the code is much more complex to handle the case if stderr and/or __stderr__ is "closed" (is NULL, None, getting closed attribute raises an error, or closed attribute is false).

4. Daemons close standard streams and fileno(stdout) can return unrelevant value.

Fixed.

[vstinner]

+ if (!closed) {
+ PyObject *res = PyObject_CallMethod(file, "close", "");
+ PyErr_Clear();
+ Py_XDECREF(res);
+ }
+ if (!dunder_closed) {
+ PyObject *res = PyObject_CallMethod(dunder_file, "close", "");
+ PyErr_Clear();
+ Py_XDECREF(res);
+ }

Hum, since it's common to have sys.__stderr__ = sys.stderr, maybe it's worth to skip the second close if dunder_file == file?

[vstinner]

@serhiy: Would you mind reviewing replace_stdio-2.patch?

[serhiy-storchaka]

I meant that C files stderr and stdout can be closed. Or it's file descriptors can be closed. I think in these cases fileno() or PyFile_NewStdPrinter() will fail.

I'm trying to consider all possible cases. Standard streams can be:

• Left original.
• Set to None.
• Reopened with different encoding/errors/etc.
• Redirected to a file (/dev/null).
• Redirected to a socket.
• Redirected to inherited file descriptor (pipe) in a subprocess.
• Be a high level wrapper around RPC (IDLE subprocess).

If the stream was reopened with the same file descriptor and closefd=True, closing it invalidates just opened "standard printer".

I would replace stdout and stderr after PyImport_Cleanup(). But PyImport_Cleanup() cleans up the sys module! Thus we should do this inside PyImport_Cleanup().

[vstinner]

I'm not sure that it's correct to replace sys.stdout with a simple object which can use a different encoding. I prefer to just close this old issue (idea).