| msg262436 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2016-03-25 13:13 |
| The Python shutdown process is complex and fragile. It was discussed in the issue #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. |
|
|
| msg262455 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-03-25 20:54 |
| 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. |
|
|
| msg262456 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-03-25 21:31 |
| 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). |
|
|
| msg262772 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-04-01 21:08 |
| 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. |
|
|
| msg262774 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-04-01 21:12 |
| + 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? |
|
|
| msg263255 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-04-12 15:46 |
| @Serhiy: Would you mind reviewing replace_stdio-2.patch? |
|
|
| msg263626 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-04-17 20:30 |
| 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(). |
|
|
| msg299192 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-07-26 02:58 |
| 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). |
|
|