Python debug build complains about thread state (was Re: SSI crashes on Win32.)

[Nicolas Lehuen <ni...@lehuen.com>]

Graham,

In fact PyEval_AcquireThread does call PyThreadState_Swap :

void
PyEval_AcquireThread(PyThreadState *tstate)
{
	if (tstate == NULL)
		Py_FatalError("PyEval_AcquireThread: NULL new thread state");
	/* Check someone has called PyEval_InitThreads() to create the lock */
	assert(interpreter_lock);
	PyThread_acquire_lock(interpreter_lock, 1);
	if (PyThreadState_Swap(tstate) != NULL)
		Py_FatalError(
			"PyEval_AcquireThread: non-NULL old thread state");
}

The error is effectively raised in the WITH_THREAD block of the
get_interpreter function :

    /* create thread state and acquire lock */
    tstate = PyThreadState_New(idata->istate);
#ifdef WITH_THREAD
    PyEval_AcquireThread(tstate);
#else
    PyThreadState_Swap(tstate);
#endif

It looks from the Python source code that this kind of way to build a
new thread state with a shared interpreter "should not be possible"...
I'm not sure I understand evrything here.

BTW, this is not related to SSI or specific to Win32, so I've changed
the subject of this thread.

Regards,
Nicolas

2006/5/20, Graham Dumpleton <gr...@dscpl.com.au>:
>
> On 20/05/2006, at 6:27 PM, Nicolas Lehuen wrote:
>
> > OK, it seems that my last hypothesis was the good one : mod_python is
> > doing things with thread states that are frowned upon by debug build.
> > The tests are only performed in the debug build, so that's why we had
> > no problem with the release build.
> >
> > The tests are found in pystate.c, in function PyThreadState_Swap (line
> > 297 in the current trunk revision) :
> >
> >       /* It should not be possible for more than one thread state
> >          to be used for a thread.  Check this the best we can in debug
> >          builds.
> >       */
> > #if defined(Py_DEBUG) && defined(WITH_THREAD)
> >       if (newts) {
> >               PyThreadState *check = PyGILState_GetThisThreadState();
> >               if (check && check->interp == newts->interp && check != newts)
> >                       Py_FatalError("Invalid thread state for this thread");
> >       }
> > #endif
> >
> > So it seems mod_python is trying to share the same thread state
> > between multiple threads, which "should not be possible". If someone
> > has an idea, please help me, because I'm not really up to date about
> > thread state management, so I'll have to read a fair bit of
> > documentation before understanding what's happening there.
>
> For each request a new thread state is created.
>
> That is, in get_interpreter() it does:
>
>      /* create thread state and acquire lock */
>      tstate = PyThreadState_New(idata->istate);
> #ifdef WITH_THREAD
>      PyEval_AcquireThread(tstate);
> #else
>      PyThreadState_Swap(tstate);
> #endif
>
> In this case, PyThreadState_Swap() isn't called when HAVE_THREAD is
> defined, which is only time your check is done.
>
> The problem seems to related to where PyThreadState_Swap() function is
> always called in make_interpreter() and PythonChildInitHandler().
>
> Will have to track down what that call is doing at those points.
> Maybe those
> calls should be:
>
> #ifdef WITH_THREAD
>              PyEval_ReleaseThread(tstate);
> #else
>              PyThreadState_Swap(NULL);
> #endif
>
> Graham
>
>
>
>
> > Regards,
> > Nicolas
> >
> > 2006/5/20, Nicolas Lehuen <ni...@lehuen.com>:
> >> I've forgot to mention the platform : as usual for me, it's
> >> Windows XP SP2.
> >>
> >> Regards,
> >> Nicolas
> >>
> >> 2006/5/20, Nicolas Lehuen <ni...@lehuen.com>:
> >> > Hi Graham,
> >> >
> >> > After a few interesting weeks, I finally manage to get some time to
> >> > help on mod_python.
> >> >
> >> > I've ran the tests on the latest Subversion revision (407968) with
> >> > Apache 2.0.58 and Python 2.4.3, and all test pass, including the
> >> SSI
> >> > ones.
> >> >
> >> > For a yet unknown reason, every test fail with a debug build of the
> >> > Python trunk with the aformentioned "Invalid thread state for this
> >> > thread" error.
> >> >
> >> > I don't know whether this is due to a mistake from me, a bug in the
> >> > Python trunk, or if it is that the debug build is a little bit more
> >> > cautious about thread state management and breaks earlier than the
> >> > non-debug version when faced by a faulty mod_python thread state
> >> > management code... I'll try to see if we still have the segfault
> >> that
> >> > could be sometimes observed when the Apache server is stopped,
> >> as it
> >> > may be related.
> >> >
> >> > Best regards,
> >> > Nicolas
> >> >
> >> > 2006/5/9, Graham Dumpleton <gr...@dscpl.com.au>:
> >> > > Nicolas Lehuen wrote ..
> >> > > > Hi Graham,
> >> > > >
> >> > > > The latest trunk version yields multiple segfaults and
> >> failures in
> >> > > > different places :
> >> > > >
> >> > > >   * Testing req.add_handler() for empty phase
> >> > > > E
> >> > > >   * Testing req.add_handler() directory
> >> > > > E
> >> > > >   * Testing interpreter per directive
> >> > > > E
> >> > > >   * Testing phase status
> >> > > > F
> >> > > >   * Testing server side include
> >> > > > F
> >> > >
> >> > > Okay Nicolas, can you check out latest and give it a go again.
> >> Got rid of
> >> > > obvious mistake of not deleting old variable definition. My
> >> checking was
> >> > > not as rigourous as it should as I should have picked up that
> >> interpreter
> >> > > name was wrong.
> >> > >
> >> > > The phase status example will probably still fail as don't
> >> understand that
> >> > > one yet. It may be an auth setup issue in test suite
> >> configuration for that
> >> > > specific test.
> >> > >
> >> > > Graham
> >> > >
> >> >
> >>
>
>