This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: segfault in call to embedded PyModule_New
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.9
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: enometh, shihai1991
Priority: normal Keywords:

Created on 2021-08-14 14:28 by enometh, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
test-case-embedded-1.zip enometh, 2021-08-14 14:28
Messages (7)
msg399591 - (view) Author: Madhu (enometh) Date: 2021-08-14 14:28
Attached zip file has a test case which illustrate the problem:

A python process (`dload.py') loads up a shared library (`libfoo1.so')
and makes a call to a foreign function `foo'. `foo' Initializes Python
and creates makes a call to PyModule_New at which point dload.py
crashes.

If the calling process is not python(`dload.c'), there is no crash

This sort of situation occurs with python-pam.  I'm not sure if this
is a programmer error and would welcome correction

[I'm supplying a zip file because I can't attach multiple files
Steps to repeat
1. compile libfoo1.so according to comment
2. Run ./dload.py
3. Optionally compile and run dload.c
msg400049 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2021-08-22 03:42
How about this one?

int foo(void)
{
  printf(stderr, "foo BEGIN\n");
  if (!Py_IsInitialized()) {
    fprintf(stderr, "python already initialized\n");
    return -1;
  }
  const char *module_name = "foo";
  PyGILState_STATE state = PyGILState_Ensure();
  PyObject *module_handle = PyModule_New(module_name);
  if (module_handle == 0)
  {
    fprintf(stderr,"PyModule_New(module_data_name=%s) failed\n",
            module_name);
    exit(1);
  }
  fprintf(stdout, "point is: %p\n", module_handle);
  fprintf(stderr, "foo END\n");
  PyGILState_Release(state);
  return 0;
}
msg400070 - (view) Author: Madhu (enometh) Date: 2021-08-22 12:26
*  hai shi <report@bugs.python.org> <682495978946.issue44913@roundup.psfhosted.org">1629603770.32.0.682495978946.issue44913@roundup.psfhosted.org>
Wrote on Sun, 22 Aug 2021 03:42:50 +0000

> hai shi <shihai1991@126.com> added the comment:

> How about this one?
>
>   if (!Py_IsInitialized()) {
>     return -1;
>   }

[When this extension is loaded into a C program rather than a python
script, then Python should be initialized here rather than quitting]

>   PyGILState_STATE state = PyGILState_Ensure();
>   PyObject *module_handle = PyModule_New(module_name);
>   if (module_handle == 0)
>   {
>     fprintf(stderr,"PyModule_New(module_data_name=%s) failed\n",
>             module_name);
>     exit(1);
>   }
>   fprintf(stdout, "point is: %p\n", module_handle);
>   fprintf(stderr, "foo END\n");
>   PyGILState_Release(state);

Yes. Thanks. Calling PyGILState_Ensure and PyGILState_Release around
the call to PyModule_New does avoid the segfault.

It is a little tricky to do this in pam-python - because although
there is one entrypoint into the extension (which can be wrapped
within the calls to PyGILState_Ensure/Release), python is initialized
within that entrypoint, but I think this can be solved another way.

Thanks
msg400119 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2021-08-23 05:09
> which can be wrapped within the calls to PyGILState_Ensure/Release.

OK, it's a good idea. But I think this enhancement will break the back 
compatibility.

> python is initialized within that entrypoint

python is initialized when you run python :)
msg400131 - (view) Author: Madhu (enometh) Date: 2021-08-23 11:45
*  hai shi <report@bugs.python.org> <481350414879.issue44913@roundup.psfhosted.org">1629695372.88.0.481350414879.issue44913@roundup.psfhosted.org>
Wrote on Mon, 23 Aug 2021 05:09:32 +0000

> hai shi <shihai1991@126.com> added the comment:
>> which can be wrapped within the calls to PyGILState_Ensure/Release.
> OK, it's a good idea. But I think this enhancement will break the back
> compatibility.

I don't understand.

Wherever I make a Py C call from the extension (libfoo1.so), I have to
wrap it up within calls to PyGILState_Ensure/Release to avoid
segfaults right?

That's how I understood your comment.

So this is on the user to avoid the segfaults, right?

And you saying my extension will not be backward compatible, i.e. it
will not work on older versions of python? (pam-python still supports
py27, I think)

Or are you saying something can be done in Python's components to
handle this use case transparently? so the user won't have to put GIL
locks in his code

(They aren't required in the normal extension case AFAICT)

>> python is initialized within that entrypoint
> python is initialized when you run python :)
[Yes, but if the extension is loaded into a C program (i.e. not from
python then the extension (libfoo1.so) has to call Py_Initialize at
that point.]
msg400133 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2021-08-23 13:05
> Or are you saying something can be done in Python's components to
handle this use case transparently? so the user won't have to put GIL
locks in his code.

Sorry, I don't explain it good enough. You have said what I wanted to say:)

> Yes, but if the extension is loaded into a C program (i.e. not from
python then the extension (libfoo1.so) has to call Py_Initialize at
that point.

You are right. I just only saw your dload.py, so I don't know how you start the python prcoess in your env;).
I haven't the experience of the pam-python:(.
msg400198 - (view) Author: Hai Shi (shihai1991) * (Python triager) Date: 2021-08-24 05:04
This bpo isn't a bug, so I close it now. If there are some enhancements need to be discussed, we can reopen it:)
History
Date User Action Args
2022-04-11 14:59:48adminsetgithub: 89076
2021-08-24 05:04:39shihai1991setstatus: open -> closed
resolution: not a bug
messages: + msg400198

stage: resolved
2021-08-23 13:05:14shihai1991setmessages: + msg400133
2021-08-23 11:45:04enomethsetmessages: + msg400131
2021-08-23 05:09:32shihai1991settype: enhancement
messages: + msg400119
2021-08-22 12:26:49enomethsetmessages: + msg400070
2021-08-22 03:42:50shihai1991setnosy: + shihai1991
messages: + msg400049
2021-08-14 14:28:01enomethcreate