-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conflict with interpreter init versus PLPython on Python 3.12 #60
Comments
I can confirm this behavior. Currently multicorn2 and PL/Python3 are mutual exclusive with Python 3.12. Once one is enabled, the second sends the server into recovery mode, regardless of the order the extensions are CREATEd. |
Wrapping in Py_IsInitialized() seems to fix this.
multicorn.c and
plpy_main.c I have no further knowledge of using Python in C, but those changes apparently allow both extensions to co-exist, regardless of load order. |
Ernst's code idea sounds good (and safe) too me. I suggest he submit a PR
and we'll merge it in the source code for 3.1 and we'll see if it proves to
be good as it sounds. In my thinkning, worst case, we back the commit
after we learn what the final solution to this challenge is.
…On Wed, Sep 25, 2024 at 9:37 AM Ernst-Georg Schmid ***@***.***> wrote:
Wrapping in (
https://docs.python.org/3/c-api/init.html#c.Py_IsInitialized)[Py_IsInitialized()]
seems to fix this.
/* Try to load plpython3 with its own module */
PG_TRY();
{
void * PyInit_plpy = load_external_function("plpython3", "PyInit_plpy", true, NULL);
if (!Py_IsInitialized()) {
PyImport_AppendInittab("plpy", PyInit_plpy);
}
need_import_plpy = true;
}
PG_CATCH();
{
need_import_plpy = false;
}
PG_END_TRY();
if (!Py_IsInitialized()) {
Py_Initialize();
}
multicorn.c
and
/* The rest should only be done once per session */
if (inited)
return;
if (!Py_IsInitialized()) {
PyImport_AppendInittab("plpy", PyInit_plpy);
Py_Initialize();
}
plpy_main.c
I have no further knowledge of using Python in C, but those changes allow
both extensions to co-exist, regardless of load order.
—
Reply to this email directly, view it on GitHub
<#60 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMWOHT3LPGNHAT32S3SRCDZYK4B7AVCNFSM6AAAAABIPV2ZO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZUGEYTGNJUGU>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
I'll get my employee (and drinkning buddy) Jan Wieck (author of PL/pgSQL,
TOAST, Referential Integrity, SLONY, etc; and a huge python fan) to also
look this over once we have it applied and it seems to work.
…On Thu, Sep 26, 2024 at 7:35 AM Denis Lussier ***@***.***> wrote:
Ernst's code idea sounds good (and safe) too me. I suggest he submit a PR
and we'll merge it in the source code for 3.1 and we'll see if it proves to
be good as it sounds. In my thinkning, worst case, we back the commit
after we learn what the final solution to this challenge is.
On Wed, Sep 25, 2024 at 9:37 AM Ernst-Georg Schmid <
***@***.***> wrote:
> Wrapping in (
> https://docs.python.org/3/c-api/init.html#c.Py_IsInitialized)[Py_IsInitialized()]
> seems to fix this.
>
> /* Try to load plpython3 with its own module */
> PG_TRY();
> {
> void * PyInit_plpy = load_external_function("plpython3", "PyInit_plpy", true, NULL);
> if (!Py_IsInitialized()) {
> PyImport_AppendInittab("plpy", PyInit_plpy);
> }
> need_import_plpy = true;
> }
> PG_CATCH();
> {
> need_import_plpy = false;
> }
> PG_END_TRY();
> if (!Py_IsInitialized()) {
> Py_Initialize();
> }
>
> multicorn.c
>
> and
>
> /* The rest should only be done once per session */
> if (inited)
> return;
>
> if (!Py_IsInitialized()) {
> PyImport_AppendInittab("plpy", PyInit_plpy);
> Py_Initialize();
> }
>
> plpy_main.c
>
> I have no further knowledge of using Python in C, but those changes allow
> both extensions to co-exist, regardless of load order.
>
> —
> Reply to this email directly, view it on GitHub
> <#60 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAMWOHT3LPGNHAT32S3SRCDZYK4B7AVCNFSM6AAAAABIPV2ZO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZUGEYTGNJUGU>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
|
I winder if we need to do something like what's proposed in #69 to handle this problem. |
My pull request #69 won't help with this issue. This one is a bit hopeless. The only way I see is forcing plypython3 to initialize before multicorn carries on with its own initialization. It's not easy because everything directly useful is
It exploits the fact that |
I think generally multicorn2 works well with Python 3.12, but the test suite cannot be executed in completeness because of the use of plpython in the write_filesystem.sql test.
multicorn2/test-3.9/sql/write_filesystem.sql
Line 36 in aebb975
When this test is executed, the backend crashes. When loaded in a debugger, it is crashing while reporting the error:
PyImport_AppendInittab() may not be called after Py_Initialize()"
, with a stack trace indicating it is being invoked from PLy_initialize in PLPython.Neither multicorn2 nor PLPython call PyImport_AppendInittab after calling Py_Initialize... multicorn2's order is correct:
multicorn2/src/multicorn.c
Lines 136 to 144 in aebb975
And Postgres' order is correct:
https://github.com/postgres/postgres/blob/8fea1bd5411b793697a4c9087c403887e050c4ac/src/pl/plpython/plpy_main.c#L116-L117
So I'm left with the initial impression that the two interpreters loaded into the backend are conflicting on each other, which frankly makes sense. There is a change in Python 3.12 (python/cpython#99402) that introduced this error condition:
https://github.com/python/cpython/pull/99402/files#diff-28cfc3e2868980a79d93d2ebdc8747dcb9231f3dd7f2caef96e74107d1ea3bf3R2691-R2694
I don't have any initial thoughts on how to avoid this conflict. Multicorn basically tries to mirror PLPython's initialization (doing a
PyImport_AppendInittab("plpy")
just because PLPython does it too), and it would be possible for it to detect if the runtime is already initialized and skip it. However, it's equally possible for PLPython to be init'd second (as in this case), in which no changes in Multicorn would prevent the error.Maybe if Multicorn could runtime detect the presence of the
PLy_initialize
symbol and invoke it... which stores a static bool for whether it has initialized the runtime... that would avoid the problem. But as it's a static function and this is C, I'm not sure that would be very feasible.I'll do some more research... but in the short-term this is the problem description at least.
The text was updated successfully, but these errors were encountered: