Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-98610: Adjust the Optional Restrictions on Subinterpreters#98618
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
gh-98610: Adjust the Optional Restrictions on Subinterpreters#98618
Uh oh!
There was an error while loading.Please reload this page.
Conversation
9de6c58 to53b5e37Compare@gpshead, we talked about this at the core sprint. |
bedevere-bot commentedOct 24, 2022
🤖 New build scheduled with the buildbot fleet by@ericsnowcurrently for commit77314bc 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
4b8c004 to6d62590Compare6d62590 toe13a305CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
minor tweaks suggested, overall good.
Lib/threading.py Outdated
| @@ -899,6 +900,8 @@ class is implemented. | |||
| self._args = args | |||
| self._kwargs = kwargs | |||
| if daemon is not None: | |||
| if daemon and not _daemon_threads_allowed(): | |||
| raise RuntimeError('daemon threads are disabled in this interpreter') | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
perhaps word the error message here and below as "daemon threads are disabled in this (sub)interpreter" to provide a better hint as to why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed
| PyDoc_STRVAR(daemon_threads_allowed_doc, | ||
| "daemon_threads_allowed()\n\ | ||
| \n\ | ||
| Return True if daemon threads are allowed in the current interpreter,\n\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why does this need to be a function? Just to prevent people from attempting to set it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Daemon threads are implemented exclusively in threading.py, so we need a way to access the info from Python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If you mean why not an attribute, it's because I was thinking of it as an interpreter setting, rather than state of the _thread module.
| @@ -0,0 +1,5 @@ | |||
| Disallowing subprocess in subinterpreters is no longer supported. Instead, | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
double negative... how about wording this like
The use of the :mod:`subprocess` is now allowed from subinterpreters. Instead:func:`os.exec` can now be disallowed. ... existing text ...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed
Uh oh!
There was an error while loading.Please reload this page.
Previously, the optional restrictions on subinterpreters were: disallow fork, subprocess, and threads. By default, we were disallowing all three for "isolated" interpreters. We always allowed all three for the main interpreter and those created through the legacy
Py_NewInterpreter()API.Those settings were a bit conservative, so here we've adjusted the optional restrictions to: fork, exec, threads, and daemon threads. The default for "isolated" interpreters disables fork, exec, and daemon threads. Regular threads are allowed by default. We continue always allowing everything For the main interpreter and the legacy API.
In the code, we add
_PyInterpreterConfig.allow_execand_PyInterpreterConfig.allow_daemon_threads. We also addPy_RTFLAGS_DAEMON_THREADSandPy_RTFLAGS_EXEC.Automerge-Triggered-By: GH:ericsnowcurrently