msg320175 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-21 13:08 |
It would help to document that calling Py_Initialize() twice in Python 3.7 now triggers a fatal error, whereas previous the second call did nothing. Document it at:
https://docs.python.org/dev/whatsnew/3.7.html#changes-in-the-c-api
... Or should it be considered as a regression?
--
My colleague Miro Hrončok reported a Python crash (SIGABRT) when running https://github.com/konlpy/konlpy test suite on Python 3.7:
"Fatal Python error: _Py_InitializeCore: main interpreter already initialized"
konlpy uses the JPype project, the bug is in JPype initialization function (it's a C extension):
* https://github.com/originell/jpype/issues/331
* https://github.com/originell/jpype/pull/332
|
msg320180 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-21 13:30 |
I set the priority to release blocker to make sure that someone looks at this issue, even if I'm not sure that it's a huge regression.
|
msg320182 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-21 13:35 |
The behaviour changed has been introduced by bpo-22257:
commit 1abcf6700b4da6207fe859de40c6c1bada6b4fec
Author: Eric Snow <ericsnowcurrently@gmail.com>
Date: Tue May 23 21:46:51 2017 -0700
bpo-22257: Private C-API for core runtime initialization (PEP 432). (#1772)
(patch by Nick Coghlan)
Simplified change:
- if (initialized)
- return;
- initialized = 1;
+ if (_Py_Initialized) {
+ Py_FatalError("Py_InitializeCore: main interpreter already initialized");
+ }
|
msg320183 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-21 13:40 |
Attached PR 7845 restores the Python 3.6 behaviour: calling Py_Initialize() twice does nothing.
|
msg320184 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-21 13:41 |
Note: the doc doesn't say that the function must only be called once, nor document the (current) new Python 3.7 restriction (must only be called once):
https://docs.python.org/dev/c-api/init.html#c.Py_Initialize
|
msg320220 - (view) |
Author: Xiang Zhang (xiang.zhang) *  |
Date: 2018-06-22 09:00 |
> Note: the doc doesn't say that the function must only be called once, nor document the (current) new Python 3.7 restriction (must only be called once):
But the doc says:
This is a no-op when called for a second time (without calling Py_FinalizeEx() first).
no-op means it could be called twice?
|
msg320223 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-22 11:13 |
"no-op means it could be called twice" oops right :-) So it's a regression. Would you mind to review my PR 7845, Xiang?
|
msg320225 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2018-06-22 11:26 |
While I agree the documentation issue means that this should be made to work again for Python 3.7.1, I'd like for Python 3.7 to at least deprecate calling Py_Initialize twice without an intervening Py_Finalize.
Otherwise the public multi-phase interpreter initialization API is going to have to cope with somebody starting the initialization process, and then calling a full Py_Initialize to finish it (instead of the new "complete initialization" API, whatever we end up calling that), and I'd prefer to keep the permitted state transitions more linear than that.
|
msg320226 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-22 11:41 |
> While I agree the documentation issue means that this should be made to work again for Python 3.7.1, I'd like for Python 3.7 to at least deprecate calling Py_Initialize twice without an intervening Py_Finalize.
In term of timeline, I would prefer to fix the regression right now, and then see how to deprecate calling the function later :-)
|
msg320227 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2018-06-22 12:01 |
Aye, I'm fine with deferring the programmatic deprecation for now - it's mainly the docs I'd like to see changed at the same time as the regression is fixed.
|
msg320244 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-22 17:14 |
New changeset 209abf746985526bce255e2fba97d3246924885d by Victor Stinner in branch 'master':
bpo-33932: Calling Py_Initialize() twice does nothing (GH-7845)
https://github.com/python/cpython/commit/209abf746985526bce255e2fba97d3246924885d
|
msg320248 - (view) |
Author: miss-islington (miss-islington) |
Date: 2018-06-22 17:33 |
New changeset 3747dd16d5d2af3499f586386e49740a0454cf44 by Miss Islington (bot) in branch '3.7':
bpo-33932: Calling Py_Initialize() twice does nothing (GH-7845)
https://github.com/python/cpython/commit/3747dd16d5d2af3499f586386e49740a0454cf44
|
msg320255 - (view) |
Author: Ned Deily (ned.deily) *  |
Date: 2018-06-22 19:12 |
Since the undocumented change in behavior from 3.6 to 3.7 appears to be causing some downstream regressions at this late stage, I think Victor's change should be cherry-picked for 3.7.0 final.
Nick or Eric, if you merge a 3.7 doc change, I'll cherry-pick that as well.
|
msg320562 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-27 09:36 |
Another example of project affected by this regression: fontforge
https://bugzilla.redhat.com/show_bug.cgi?id=1595421
|
msg320578 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2018-06-27 13:52 |
https://github.com/python/cpython/pull/7967 amends the docs and adds the deprecation notices, but after taking another look at the way Victor's patch works, I'm thinking we may not need them: the *new* initialization API remains strict about the required state machine transitions, which means that calling `Py_Initialize` when only the core has been initialized will fail at the _Py_InitializeCore step (as that complains if the core has already been initialized).
Similarly, calling _Py_InitializeCore after Py_Initialize still fails, as the *only* case that becomes a no-op is specifically Py_InitializeEx, *after* the interpreter is fully initialized.
Given that, I think we can just keep Victor's compatibility fix indefinitely, and only have the new incremental initialization APIs be strict about the expected transitions through the state machine.
|
msg320579 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2018-06-27 13:55 |
I went ahead and closed my PR, as I'm now happy that keeping this behaviour isn't going to block anything we want to do for PEP 432.
If we change our minds and decide we want to deprecate the current behaviour after all, then we can start that deprecation process in Python 3.8.
|
msg320582 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-27 14:22 |
Please keep it open until 3.7.0 is released.
|
msg320634 - (view) |
Author: Ned Deily (ned.deily) *  |
Date: 2018-06-27 22:34 |
New changeset b940921a67fa33e8c8812aa3b7746161c2552c1d by Ned Deily (Miss Islington (bot)) in branch '3.7':
bpo-33932: Calling Py_Initialize() twice does nothing (GH-7845)
https://github.com/python/cpython/commit/b940921a67fa33e8c8812aa3b7746161c2552c1d
|
msg320641 - (view) |
Author: Ned Deily (ned.deily) *  |
Date: 2018-06-28 01:29 |
Fixed in 3.7.0. Thanks, everyone!
|
msg320665 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-28 12:42 |
I reopen the issue because fontforge still fails on Python 3.7.0 final. fontforge calls Py_Initialize() and then calls Py_Main() which fails with an assertion: https://bugzilla.redhat.com/show_bug.cgi?id=1595421
The same code works well on Python 3.6, so should we define this issue as a regression?
Call stack:
* Py_Main()
* pymain_main()
* _Py_InitializeCore() which fails
The problem is that the first call of Py_Initialize() sets a first configuration, but then Py_Main() sets a different configuration.
Is it correct to call Py_Initialize() before Py_Main()?
|
msg320666 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-28 12:49 |
I looked quickly at _Py_InitializeCore() and I'm not sure that it's designed to replace an existing configuration.
Example:
int Py_HashRandomizationFlag = 0; /* for -R and PYTHONHASHSEED */
(...)
if (!core_config->use_hash_seed || core_config->hash_seed) {
/* Random or non-zero hash seed */
Py_HashRandomizationFlag = 1;
}
If Py_Initialize() sets Py_HashRandomizationFlag to 1, but Py_Main() doesn't use an hash seed, Py_HashRandomizationFlag value remains 1 which is wrong.
pymain_read_conf() fills _PyCoreConfig from global configuration flags, but then global configuration flags are supposed to be set from the global configuration flags.
So maybe for this specific case, _Py_InitializeCore() should always set Py_HashRandomizationFlag. But it was just one example to show that right now, _Py_InitializeCore() doesn't seem to support to be called twice with two different configurations.
|
msg320667 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-06-28 12:51 |
Ok, now with my regression hat.
In Python 3.6, Py_Main() calls Py_Initialize(). If Py_Initialize() is called twice, the second call does nothing. So it's fine to call Py_Main() after Py_Initialize() in Python 3.6.
|
msg320725 - (view) |
Author: Eric Snow (eric.snow) *  |
Date: 2018-06-29 18:12 |
> I looked quickly at _Py_InitializeCore() and I'm not sure
> that it's designed to replace an existing configuration.
> ...
> So maybe for this specific case, _Py_InitializeCore() should
> always set Py_HashRandomizationFlag.
+1
At the same time, the use of Py_*Flag variables is a little unclear in the PEP 432 world. Under the PEP I'd expect code in CPython to use the core config from the current PyInterpreterState (or even _PyRuntimeState), rather than the global flag variables. So presumably (the PEP doesn't talk about it) the intent of keeping them is to help folks that currently make use of the flags. However, they shouldn't be modifying them after initialization, and under PEP 432 they don't need to set them beforehand (they set values on the config), right?
Part of the problem here is that Py_Main() results in all the flag variables getting set (before runtime initialization). However, with the current PEP 432 [internal] implementation those variables are (mostly) never getting set, right? Shouldn't we set all of them, but only at the end of _Py_InitializeCore() (for the sake of extensions/embedders that use them)? In that case we'd need to replace our internal use of Py_HashRandomizationFlag with field on the config struct on _PyRuntimeState.
> But it was just one example to show that right now,
> _Py_InitializeCore() doesn't seem to support to be called
> twice with two different configurations.
Are there other things (than how we set that flag) that break in that case? Regardless, what would ensure that it works properly?
|
msg320755 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2018-06-30 06:40 |
Victor, could you file a separate issue for Py_Main(). It's a different problem, since Py_Main() is an entry point for an *application that embeds Python*. It's not a CPython API in the tradition sense.
|
msg320759 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2018-06-30 07:01 |
I filed https://bugs.python.org/issue34008 to cover the fact that "Py_Initialize(); Py_Main();" doesn't work in 3.7.0 whereas it used to sort of work (by only partially applying the settings read from the CLI and environment by the Py_Main() call).
Since "Py_Initialize(); Py_Initialize();" *does* work in 3.7.0, closing this one again.
|
msg320761 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2018-06-30 07:17 |
Answering Eric's question about the flag variables though:
* pymain_get_global_config copies the global variable flags to their respective struct entries
* pymain_set_global_config copies the various struct entries to their respective global variable flags
One of the first things that Py_Main() does in 3.7 is to:
- read the global variable flags into the relevant structs
- update the structs based on the CLI options
- write the struct values back to the global flags
This mostly fakes the 3.6-and-earlier behaviour of using the global variables directly.
For anything listed in https://docs.python.org/3/c-api/init.html#global-configuration-variables, we also mostly left any code reading the global variable directly alone, since they're considered a public API for embedding applications to dynamically adjust behaviour and we don't have a good way of deprecating using them for that purpose :(
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:02 | admin | set | github: 78113 |
2018-06-30 07:17:14 | ncoghlan | set | messages:
+ msg320761 |
2018-06-30 07:01:22 | ncoghlan | set | status: open -> closed resolution: fixed messages:
+ msg320759
|
2018-06-30 06:40:07 | ncoghlan | set | messages:
+ msg320755 |
2018-06-29 18:12:17 | eric.snow | set | nosy:
+ emilyemorehouse messages:
+ msg320725
|
2018-06-28 13:17:49 | hroncok | set | nosy:
+ hroncok
|
2018-06-28 12:51:42 | vstinner | set | messages:
+ msg320667 |
2018-06-28 12:49:55 | vstinner | set | messages:
+ msg320666 |
2018-06-28 12:42:02 | vstinner | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg320665
|
2018-06-28 01:29:46 | ned.deily | set | status: open -> closed priority: release blocker -> resolution: fixed messages:
+ msg320641
|
2018-06-27 22:34:50 | ned.deily | set | messages:
+ msg320634 |
2018-06-27 14:22:58 | vstinner | set | status: closed -> open resolution: fixed -> (no value) messages:
+ msg320582
|
2018-06-27 13:55:18 | ncoghlan | set | status: open -> closed type: behavior messages:
+ msg320579
resolution: fixed stage: patch review -> resolved |
2018-06-27 13:52:30 | ncoghlan | set | messages:
+ msg320578 |
2018-06-27 13:40:31 | ncoghlan | set | pull_requests:
+ pull_request7574 |
2018-06-27 09:36:18 | vstinner | set | messages:
+ msg320562 |
2018-06-22 19:12:47 | ned.deily | set | messages:
+ msg320255 |
2018-06-22 17:33:51 | miss-islington | set | nosy:
+ miss-islington messages:
+ msg320248
|
2018-06-22 17:16:19 | miss-islington | set | pull_requests:
+ pull_request7469 |
2018-06-22 17:14:54 | vstinner | set | messages:
+ msg320244 |
2018-06-22 12:01:06 | ncoghlan | set | messages:
+ msg320227 |
2018-06-22 11:41:27 | vstinner | set | messages:
+ msg320226 |
2018-06-22 11:26:55 | ncoghlan | set | messages:
+ msg320225 |
2018-06-22 11:13:28 | vstinner | set | messages:
+ msg320223 |
2018-06-22 09:00:37 | xiang.zhang | set | nosy:
+ xiang.zhang messages:
+ msg320220
|
2018-06-21 13:41:11 | vstinner | set | messages:
+ msg320184 |
2018-06-21 13:40:20 | vstinner | set | messages:
+ msg320183 |
2018-06-21 13:35:14 | vstinner | set | messages:
+ msg320182 components:
+ Interpreter Core, - Documentation |
2018-06-21 13:31:43 | vstinner | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request7455 |
2018-06-21 13:30:53 | vstinner | set | priority: normal -> release blocker
messages:
+ msg320180 |
2018-06-21 13:08:48 | vstinner | create | |