The sys module is a rather special case as far as modules go. It is effectively a "console" into the interpreter's internal state and that includes some mutable state. Since it is a module, we don't have much of an opportunity to: * validate values assigned to its attributes [1] * issue DeprecationWarning for deprecated attrs [2] * alias attrs [2] * replace get (and get/set) functions with properties * re-organize sys [3] One possible solution I've been toying with for quite a while [2] is to rename the current sys module "_sys" and then add Lib/sys.py. The new module would proxy the old one (for backward-compatibility), but also allow us to do all of the above (e.g. validate sys.modules). I implemented this a few weeks ago: https://github.com/ericsnowcurrently/cpython/tree/sys-module (for the sake of comparison: https://github.com/ericsnowcurrently/cpython/pull/2) It uses the trick of replacing itself in sys.modules (though it could just as well set __class__). The only problem I've encountered is code that uses "type(sys)" to get types.ModuleType. Under my branch "type(sys)" returns the ModuleType subclass. Otherwise everything looks fine. Thoughts? -eric [1] I ran into this recently: https://bugs.python.org/issue31404 [2] I have plans for a low-level encapsulation of the import state and a high-level API for the current import machinery as a whole. As part of that I'd like to be able to deprecate the current import state attrs (e.g. sys.modules) and make them look up values from sys.importstate. [3] This has come up before, including https://www.python.org/dev/peps/pep-3139/. Also PEP 432 implies a clearer structure for sys.
On 13 September 2017 at 09:46, Eric Snow <ericsnowcurrently@gmail.com> wrote:
The sys module is a rather special case as far as modules go. It is effectively a "console" into the interpreter's internal state and that includes some mutable state. Since it is a module, we don't have much of an opportunity to:
* validate values assigned to its attributes [1] * issue DeprecationWarning for deprecated attrs [2] * alias attrs [2] * replace get (and get/set) functions with properties * re-organize sys [3]
+1 from me, specifically because there are edge cases we don't generally test (e.g. folks rebinding sys.modules to nonsense), and it would be nice to be able to upgrade those from "don't do that" to "the obvious way of doing that just plain isn't allowed". Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
I find this a disturbing trend. I think we have bigger fish to fry and this sounds like it could slow down startup. On Tue, Sep 12, 2017 at 8:35 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
On 13 September 2017 at 09:46, Eric Snow <ericsnowcurrently@gmail.com> wrote:
The sys module is a rather special case as far as modules go. It is effectively a "console" into the interpreter's internal state and that includes some mutable state. Since it is a module, we don't have much of an opportunity to:
* validate values assigned to its attributes [1] * issue DeprecationWarning for deprecated attrs [2] * alias attrs [2] * replace get (and get/set) functions with properties * re-organize sys [3]
+1 from me, specifically because there are edge cases we don't generally test (e.g. folks rebinding sys.modules to nonsense), and it would be nice to be able to upgrade those from "don't do that" to "the obvious way of doing that just plain isn't allowed".
Cheers, Nick.
-- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia _______________________________________________ Python-ideas mailing list Python-ideas@python.org https://mail.python.org/mailman/listinfo/python-ideas Code of Conduct: http://python.org/psf/codeofconduct/
-- --Guido van Rossum (python.org/~guido)
On Tue, Sep 12, 2017 at 9:30 PM, Guido van Rossum <guido@python.org> wrote:
I find this a disturbing trend.
Which trend? Moving away from "consenting adults"? In the case of sys.modules, the problem is that assigning a bogus value (e.g. []) can cause the interpreter to crash. It wasn't a problem until recently when I removed PyInterpreterState.modules and made sys.modules authoritative (see https://bugs.python.org/issue28411). The options there are: 1. revert that change (which means assigning to sys.modules deceptively does nothing) 2. raise an exception in all the places that expect sys.modules to be a mapping (far from where sys.modules was re-assigned) 3. raise an exception if you try to set sys.modules to a non-mapping 4. let a bogus sys.modules break the interpreter (basically, tell people "don't do that") My preference is #3 (obviously), but it sounds like you'd rather not.
I think we have bigger fish to fry and this sounds like it could slow down startup.
It should have little impact on startup. The difference is the cost of importing the new sys module (which we could easily freeze to reduce the cost). That cost would apply only to programs that currently import sys. Everything in the stdlib would be updated to use _sys directly. If you think it isn't worth it then I'll let it go. I brought it up because I consider it a cheap, practical solution to the problem I ran into. Thanks! -eric
My preference is (1), revert. You have to understand how sys.modules works before you can change it, and if you waste time debugging your mistake, so be it. I think the sys.py proposal is the wrong way to fix the mistake you made there. On Wed, Sep 13, 2017 at 4:00 PM, Eric Snow <ericsnowcurrently@gmail.com> wrote:
On Tue, Sep 12, 2017 at 9:30 PM, Guido van Rossum <guido@python.org> wrote:
I find this a disturbing trend.
Which trend? Moving away from "consenting adults"? In the case of sys.modules, the problem is that assigning a bogus value (e.g. []) can cause the interpreter to crash. It wasn't a problem until recently when I removed PyInterpreterState.modules and made sys.modules authoritative (see https://bugs.python.org/issue28411). The options there are:
1. revert that change (which means assigning to sys.modules deceptively does nothing) 2. raise an exception in all the places that expect sys.modules to be a mapping (far from where sys.modules was re-assigned) 3. raise an exception if you try to set sys.modules to a non-mapping 4. let a bogus sys.modules break the interpreter (basically, tell people "don't do that")
My preference is #3 (obviously), but it sounds like you'd rather not.
I think we have bigger fish to fry and this sounds like it could slow down startup.
It should have little impact on startup. The difference is the cost of importing the new sys module (which we could easily freeze to reduce the cost). That cost would apply only to programs that currently import sys. Everything in the stdlib would be updated to use _sys directly.
If you think it isn't worth it then I'll let it go. I brought it up because I consider it a cheap, practical solution to the problem I ran into. Thanks!
-eric
-- --Guido van Rossum (python.org/~guido)
Le 14 sept. 2017 01:01, "Eric Snow" <ericsnowcurrently@gmail.com> a écrit : In the case of sys.modules, the problem is that assigning a bogus value (e.g. []) can cause the interpreter to crash. It wasn't a problem until recently when I removed PyInterpreterState.modules and made sys.modules authoritative (see https://bugs.python.org/issue28411). How do you crash Python? Can't we fix the interpreter? Your change makes so I would prefer to keep it if possible. Victor
On Thu, Sep 14, 2017 at 4:26 PM, Victor Stinner <victor.stinner@gmail.com> wrote:
How do you crash Python?
See https://bugs.python.org/issue31404.
Can't we fix the interpreter?
I'm looking into it. In the meantime I've split the original branch up into 3. The first I've already landed. [1] The second I'll land once I resolve some refcount leaks. [2] The final branch is the one that actually drops PyInterpreterState.modules. [3] It's quite small, but that's the part that causes the crash. So we'll have to adapt it if we want to make it work before it can be merged again (or else we'll be right back where we were before I reverted).
Your change makes so I would prefer to keep it if possible.
Why in particular do you want to keep the change? -eric [1] https://github.com/python/cpython/pull/3575 [2] https://github.com/python/cpython/pull/3593 [3] https://github.com/ericsnowcurrently/cpython/compare/sys-modules-any-mapping...
participants (4)
-
Eric Snow -
Guido van Rossum -
Nick Coghlan -
Victor Stinner