[pjsip] Deadlocks: solved

Turnaev Eugeny turnaev at t72.ru
Tue Jul 8 07:34:16 EDT 2008


On Tuesday 08 July 2008 15:58:52 Benny Prijono wrote:

> >        I have commented
> >        lines Py_BEGIN_ALLOW_THREADS
> >        and Py_END_ALLOW_THREADS

> >
> Thanks for the info. Frankly I've forgotten why those macros are there, but
> reading the code comments there I'm not too sure if removing it is a good
> idea. According to the comments, and if I recall correctly, without those
> macros the readline() call (used by pjsua_app.py to operate console menu)
> will block forever. So are you sure that's a good idea?
> 

	Well i think calling sys.stdin.readline() should block and it is normal 	behavior
	of blocking IO call.
	I am not getting you here.
	Py_BEGIN_ALLOW_THREADS must be called to make it possible other python threads
	to run.. a typical situation where it is called - is some IO or IO-like operations.
	For example:

	Py_BEGIN_ALLOW_THREADS

		// execute some time consuming database sql select..
		// the point is while you fetching sql rows from disk -> to allow 
		// other python threads to run.

		*database_cursor_pointer->execute;	

	Py_END_ALLOW_THREADS

	if you working with stdin in blocking mode and called
	readline() -> that will block thread where it is called 
	no matter if you allow this thread to run with Py_BEGIN_ALLOW_THREADS
	in other thread.
	

> If so then probably we can also remove
> PyGILState_Ensure()/PyGILState_Release() call in
> ENTER_PYTHON()/LEAVE_PYTHON() macros.
> 
	Ok. as it said in python documentation
	PyGILState_Ensure()/ Release() is needed to
	call python C API from not registered in python C thread.
	So i am pretty sure it is required in callbacks that are called from C pjsua threads.


> One thing for sure, I was unsure about which threading model works best when
> I created py_pjsua.c, and I experimented with several threading models. So
> it could be that some of these are artifacts from that experimentations.
> 
> 

	
	I am also don't getting this:

static void cb_log_cb(int level, const char *data, int len)
{
    /* Ignore if this callback is called from alien thread context,
     * or otherwise it will crash Python.
     */
    if (pj_thread_local_get(thread_id) == 0)
        return;

    if (PyCallable_Check(obj_log_cb))
    {
        ENTER_PYTHON();

        PyObject_CallFunctionObjArgs(
            obj_log_cb, Py_BuildValue("i",level),
            PyString_FromString(data), Py_BuildValue("i",len), NULL
        );

        LEAVE_PYTHON();
    }
}

	Hmm.. anyway cb_log_cb is called from C thread of pjsia..
	I have no calls to py_pjsua.log_cb in my app, So i suppose 
	cb_log_cb is always called from an alien thread, there is no point
	in checking if thread is alien.

	Well i am not guru in python threading model, a half hour ago i read
	about PyGILState_Ensure()/PyGILState_Release().. but it seems
	that it can not crash python as long as you make all calls to python
	api inside PyGILState_Ensure()/PyGILState_Release().

	I think PyGILState_Ensure registers you current thread (python now knows about your C thread) 
	and saves the state of previous thread that was run in python to restore when PyGILState_Release is called,
	and when release is called your C thread is also unregistered by python... python
	now knows nothing about your C thread.
	

 
> >        Also i get rid of worker_thread to poll with handle_events() .. now
> > i am
> >        polling from the thread where all other calls to py_pjsua lib is
> > located.
> >
> >
> That would be the best way indeed. But now that you invoke pjsua from one
> thread, the original handle_events() (with Py_BEGIN_ALLOW_THREADS) should
> work, shouldn't it?
> 
> 
	Well yes it should work, i am just not getting the purpose of Py_BEGIN_ALLOW_THREADS there.


> 
> Yep, normally that shouldn't be a problem, assuming that application doesn't
> have it's own mutex. Of course in this case the application, i.e. Python,
> does have a mutex, so this what might have caused the problem (probably
> because Py_BEGIN_ALLOW_THREADS releases Python mutex?).
> 
> 
	Well Py_BEGIN_ALLOW_THREADS just makes other python threads to run,
	it should be safe unless
		pjsua_handle_events(msec) modifies some structures that python threads uses.
		Also it is unsafe in case where 
			lock1 acquired in pjsua_handle_events and lock2 is going to be acquired
		while other python thread allowed to run
			already acquired lock2 and going to acquire lock2.
		a standard deadlock situation.
		But i dont think this happens, modifying some structures that python uses in pjsua_handle_events
		is more probably, or some structures that uses pjsua.

		
		

> 
> Yeah, could be. I'm now also working on the wrapper again so I'll look into
> that.
> 
> 

	What threading design will be in a new wrapper? 


> >
> >        Now i have no worker thread... i am polling right from the thread
> > where all other
> >        calls to pj_pjsua located and also removed Py_BEGIN_ALLOW_THREADS
> > from handle_events
> >        to disallow interleave of threads while i am in handle_events().
> >
> >
> Try putting sleep() in the other thread (the one that doesn't call pjsua),
> and see if that thread block forever. ;-)
> 
	Well actually it blocked in putting call task to python Queue object,
	and caller thread (the one that calls py_pjsua api) gets that task from queue.
	But when it is not blocked in Queue.put() method it works fine, i can see debug messages
	in log from this thread.

	Again: Py_BEGIN_ALLOW_THREADS is used to allow other python thread
	to run while you are in some blocking OS call or in some time consuming IO operations that do not
	require processor time.

> 
> I'm now working on object oriented Python wrapper on top of the C Python
> module, hopefully that will make it easier to use. Stay tuned! :)
> 

Whoa cool :)

Could you please give an example of threading model/api that will be used in
a new wrapper? Can we discuss it? Maybe some my notes will help ;)






More information about the pjsip mailing list