[Libguestfs] [PATCH NOT WORKING nbdkit 0/3] python: Allow thread model to be set from Python plugins.

[Daniel P. Berrangé]

On Wed, Aug 05, 2020 at 04:40:26PM +0100, Richard W.M. Jones wrote:
> Patch 2 certainly allows you to set the thread model.  However patch 3
> shows that if you set it to nbdkit.THREAD_MODEL_PARALLEL it will
> crash.
> 
> If you look closely at the stack trace (attached below) you can see
> that ignoring threads which are in parts of nbdkit unrelated to
> Python:
> 
> Thread 4: In pread, waiting in time.sleep().  This thread has released
>           the GIL.

Has it - I'm not seeing any call to PyEval_SaveThread()/RestoreThread
which is needed to release the GIL when going from Python to C.

> 
> Thread 2: Started to process a pread call but didn't reach Python code yet.
> 
> Thread 1: In pread, segfaults when checking if pread() is defined
>           in the Python code.
> 
> My understanding is this should all be OK and there's no reason for
> Python to crash here.  I wonder if it's because we're calling "down"
> into Python from C, rather than the usual way of calling from Python
> into C.

I'm also not seeing any coodes to PyGILState_Ensure()/Release which is
needed call into Python from C.

Without these calls, random crashes are certainly expected.

What libvirt-python does is 

eg

    PyThreadState *_save = NULL;
    if (PyEval_ThreadsInitialized())  
      _save = PyEval_SaveThread();

    ...call C functions...

    if (PyEval_ThreadsInitialized())
       PyEval_RestoreThread(_save);


and 

    PyGILState_STATE _save = PyGILState_UNLOCKED;
    
    if (PyEval_ThreadsInitialized())
      _save = PyGILState_Ensure();

    ... call Python functions...

    if (PyEval_ThreadsInitialized())
      PyGILState_Release(_save);

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|