I was thinking that if APR_HAS_THREADS was 0, then Apache was forcibly ran in prefork mode, so there was no need for thread safety at all, given the fact that mod_python would only run one interpreter thread. So if WITH_THREAD was not defined, ORAPR_HAS_THREADS was 0, then we would not need any thread safety code. Hence the definition of MOD_PYTHON_WITH_THREAD_SUPPORT.
You're right in writing that a user could launch a new thread in Python, provided that WITH_THREAD is defined, even if APR_HAS_THREADS==0. However, having a look at the parts of mod_python.c where the thread safety was put in, I think we can safely say that those parts are only called by mod_python (through python_handler, python_cleanup etc who call get_interpreter). Those parts are therefore always called in the same thread (if APR_HAS_THREADS==0, that is) and there is no need for thread synchronization to be done (no shared data between the main thread and the other user threads, no need to release the GIL etc.).
BUT, I could be very, very wrong here, and your idea of reverting to a conservative "shield python threading calls with WITH_THREAD and apr threading code with APR_HAS_THREADS" is way more attractive to my tired mind right now. So if you want I can revert all this MOD_PYTHON_WITH_THREAD_SUPPORT hack.
Regards,
Nicolas
2005/9/12, Gregory (Grisha) Trubetskoy <[EMAIL PROTECTED]>:
I'm not sure I understand this, perhaps someone could write a message to
the list explaining what we're doing here so there is a record. Sorry if
I'm being slow-headed here.
To me it seems that when you use thread-related calls from Python, you
wrap those in Python defines (WITH_THREAD) and when you use thread-related
calls from APR, you wrap those in APR defines (APR_HAS_THREAD), and that's
all?
In other words - what does MOD_PYTHON_WITH_THREAD_SUPPORT accomplish that
the above does not.
Also, given:
#if(defined(WITH_THREAD) && APR_HAS_THREADS)
#define MOD_PYTHON_WITH_THREAD_SUPPORT 1
#else
#define MOD_PYTHON_WITH_THREAD_SUPPORT 0
#endif
Does this mean that if Python is compiled with thread support and APR is
not, MOD_PYTHON_WITH_THREAD_SUPPORT is 0 which means that the thread
safety code isn't there, but you still _can_ create threads because Python
will let you - isn't this asking for a segfault/deadlock/whatever?
Grisha
On Mon, 12 Sep 2005, Jim Gallacher wrote:
> Gregory (Grisha) Trubetskoy wrote:
>>
>> Shouldn't that be PYTHON_WITH_THREAD rather than MOD_PYTHON_WITH_THREAD?
>>
>
> I understand it to mean that we want the thread handling code compiled into
> mod_python.
>
> Compiling and testing right now.
>
> Jim
>
>>
>> On Mon, 12 Sep 2005, Nicolas Lehuen wrote:
>>
>>> I've checked in a changeset wherein I define
>>> MOD_PYTHON_WITH_THREAD_SUPPORT
>>> and use it everywhere WITH_THREAD was previously used. This should do
>>> the
>>> trick ! Now if someone (like Jim) can give us his +1, that would be
>>> great.
>>>
>>> Regards,
>>> Nicolas
>>>
>>> 2005/9/12, Gregory (Grisha) Trubetskoy < [EMAIL PROTECTED]>:
>>>
>>>>
>>>>
>>>> Just wanted to add to this message that if Jim's version runs and
>>>> tests
>>>> with the trick below (envvars is executed prior to apache start, but I
>>>> don't think the tests use it, so you'll probably just have to set this
>>>> var
>>>> in the shell in which the tests are run), then this would be a
>>>> solution
>>>> for all FreeBSD issues and we could roll a beta 3 which will have a
>>>> great
>>>> change of being publicly released.
>>>>
>>>> Grisha
>>>>
>>>> On Mon, 12 Sep 2005, Gregory (Grisha) Trubetskoy wrote:
>>>>
>>>>>
>>>>> OK, found it. This should work on FreeBSD where Python is threaded
>>>>> and
>>>>
>>>> Apache
>>>>
>>>>> is not.
>>>>>
>>>>> [snip]
>>>>>
>>>>> And, if you built apache without thread support, you may need to add
>>>>> the
>>>>> following lines to $PREFIX/sbin/envvars:
>>>>>
>>>>> LD_PRELOAD=/usr/lib/libc_r.so
>>>>> export LD_PRELOAD
>>>>>
>>>>> [snip]
>>>>>
>>>>>
>>>>> On Mon, 12 Sep 2005, Gregory (Grisha) Trubetskoy wrote:
>>>>>
>>>>>>
>>>>>> On Mon, 12 Sep 2005, Jim Gallacher wrote:
>>>>>>
>>>>>>> *** Warning: Linking the shared library mod_python.la against
>>>>>>> the
>>>>>>> *** static library
>>>>>>> /usr/local/lib/python2.4/config/libpython2.4.a is
>>>>
>>>> not
>>>>
>>>>>>> portable!
>>>>>>
>>>>>>
>>>>>> I think this was always there and its pretty harmless.
>>>>>>
>>>>>>> On qemu:
>>>>>>> Syntax error on line 44 of
>>>>>>> /usr/home/jim/tmp/mod_python/test/conf/test.conf:
>>>>>>> Cannot load /usr/home/jim/tmp/mod_python/src/mod_python.so into
>>>>
>>>> server:
>>>>
>>>>>>> /usr/home/jim/tmp/mod_python/src/mod_python.so: Undefined symbol
>>>>>>> "pthread_attr_init"
>>>>>>
>>>>>>
>>>>>>
>>>>>> This is because FreeBSD's libc comes in two versions - threaded
>>>>>> and
>>>>>> non-threaded. If Python is linked against the threaded ones and
>>>>>> Apache
>>>>>> against the non-thrreaded, then you get this problem. There is a
>>>>>> simple
>>>>>> fix for this - you just cause Apache to start with threaded libs,
>>>>>> but I
>>>>>> can't find any references to it right now and have to run off to a
>>>>>> meeting.
>>>>>>
>>>>>> Grisha
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> It is quite possible I don't have things configured correctly on
>>>>>>> the
>>>>>>> QEMU version and hence the different undefined symbol but it
>>>>>>> doesn't
>>>>>>> really matter since it fails either way. I don't have time to
>>>>>>> investigate further right now. I'll revisit this tonight.
>>>>>>>
>>>>>>> Regards,
>>>>>>> Jim
>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Nicolas
>>>>>>>> #if APR_HAS_THREADS && defined(WITH_THREAD)
>>>>>>>> 2005/9/11, Jim Gallacher < [EMAIL PROTECTED]
>>>>>>>> <mailto:[EMAIL PROTECTED]>>:
>>>>>>>>
>>>>>>>> FYI, I found the following note in the INSTALL file in the
>>>>>>>> apache
>>>>>>>> source:
>>>>>>>>
>>>>>>>> * If you are building on FreeBSD, be aware that threads will
>>>>>>>> be disabled and the prefork MPM will be used by default,
>>>>>>>> as threads do not work well with Apache on FreeBSD. If
>>>>>>>> you wish to try a threaded Apache on FreeBSD anyway, use
>>>>>>>> "./configure --enable-threads".
>>>>>>>>
>>>>>>>> I'm also setting up FreeBSD under QEMU... so far so good, but
>>>>>>>> installing
>>>>>>>> anything using ports is really slow. QEMU's performance here
>>>>>>>> is
>>>>>>>> just
>>>>>>>> killing me. I guess I should have read the manual first and
>>>>>>>> used
>>>>>>>> the
>>>>>>>> binary packages for the software I wanted to install. :-(
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Jim
>>>>>>>>
>>>>>>>> Jim Gallacher wrote:
>>>>>>>>
>>>>>>>>> Nicolas Lehuen wrote:
>>>>>>>>>
>>>>>>>>>> OK, I've checked in a version that compiles both on at
>>>>>>>>>> least
>>>>>>>>
>>>>>>>> Win32 and
>>>>>>>>
>>>>>>>>>> FreeBSD. I'm just testing if APR_HAS_THREAD is defined and
>>>>>>>>
>>>>>>>> only
>>>>>>>>
>>>>>>>>>> include the apr_thread_mutex_lock and unlock calls if it
>>>>>>>>>> is
>>>>>>>>
>>>>>>>> defined.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Compiles a passes unit tests on Linux Debian sid with
>>>>>>>>
>>>>>>>> mpm-prefork.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Now, on minotaur, APR_HAS_THREAD is defined as 0. Does
>>>>>>>>>> this
>>>>>>>>
>>>>>>>> mean
>>>>>>>> that
>>>>>>>>
>>>>>>>>>> Apache is not configured for threading ? Can we assume
>>>>>>>>>> that we
>>>>>>>>
>>>>>>>> are in
>>>>>>>>
>>>>>>>>>> the prefork model if APR_HAS_THREAD==0, so that we can
>>>>>>>>>> skip
>>>>>>>>
>>>>>>>> all the
>>>>>>>>
>>>>>>>>>> locking code ? Because that's what we do right now.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Debian sid with apache2.0.54 mpm-prefork, APR_HAS_THREAD
>>>>>>>>> ==
>>>>>>>>
>>>>>>>> 1.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Jim
>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Nicolas
>>>>>>>>>>
>>>>>>>>>> 2005/9/11, Nicolas Lehuen < [EMAIL PROTECTED]
>>>>>>>>
>>>>>>>> <mailto:[EMAIL PROTECTED] >
>>>>>>>>
>>>>>>>>>> <mailto: [EMAIL PROTECTED]
>>>>>>>>
>>>>>>>> <mailto: [EMAIL PROTECTED]>>>:
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Yes, this new code is something I commited on the
>>>>>>>>
>>>>>>>> 29/12/2004
>>>>>>>> (I used
>>>>>>>>
>>>>>>>>>> the "blame" function of TortoiseSVN for that). It was a
>>>>>>>>
>>>>>>>> patch by
>>>>>>>>
>>>>>>>>>> Graham to fix MODPYTHON-2
>>>>>>>>>> < http://issues.apache.org/jira/browse/MODPYTHON-2>.
>>>>>>>>>>
>>>>>>>>>> The problem is not in the patch, but rather in the fact
>>>>>>>>
>>>>>>>> that
>>>>>>>> APR
>>>>>>>>
>>>>>>>>>> seems configured without the thread support while Python
>>>>>>>>
>>>>>>>> is
>>>>>>>>
>>>>>>>>>> configured with thread support. mod_python.c assumes that
>>>>>>>>
>>>>>>>> is
>>>>>>>>
>>>>>>>>>> WITH_THREAD is defined, then the APR mutex functions are
>>>>>>>>
>>>>>>>> available,
>>>>>>>>
>>>>>>>>>> which is wrong. Maybe we should test for APR_HAS_THREADS
>>>>>>>>
>>>>>>>> instead ?
>>>>>>>>
>>>>>>>>>> In that case, won't this cause any problems on threaded
>>>>>>>>
>>>>>>>> platforms ?
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I don't know if this is a problem specific to minotaur or
>>>>>>>>
>>>>>>>> to
>>>>>>>> all
>>>>>>>>
>>>>>>>>>> version of FreeBSD. I'm currently downloading the ISOs of
>>>>>>>>
>>>>>>>> FreeBSD
>>>>>>>>
>>>>>>>>>> and I'll try using QEMU to run a FreeBSD setup on my
>>>>>>>>
>>>>>>>> computer, but
>>>>>>>>
>>>>>>>>>> that will be long and troublesome. If someone has more
>>>>>>>>
>>>>>>>> clue
>>>>>>>> on this
>>>>>>>>
>>>>>>>>>> issue, feel free to tell us :).
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Nicolas
>>>>>>>>>>
>>>>>>>>>> 2005/9/10, Jim Gallacher <[EMAIL PROTECTED]
>>>>>>>>
>>>>>>>> <mailto:[EMAIL PROTECTED]>
>>>>>>>>
>>>>>>>>>> <mailto: [EMAIL PROTECTED]
>>>>>>>>
>>>>>>>> <mailto:[EMAIL PROTECTED]>>>:
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> I'm stubling around in the dark here, but maybe this
>>>>>>>>>>> will
>>>>>>>>
>>>>>>>> create a
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> spark
>>>>>>>>>>
>>>>>>>>>>> of an idea. I took a diff of mod_python.c from 3.1.4 and
>>>>>>>>
>>>>>>>> 3.2.1b and
>>>>>>>>
>>>>>>>>>>> isolated the lines which correspond to the compilation
>>>>>>>>
>>>>>>>> error.
>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Compiler messages
>>>>>>>>>>> -----------------
>>>>>>>>>>>
>>>>>>>>>>> mod_python.c:34: error: syntax error before '*' token
>>>>>>>>>>> mod_python.c:34: warning: type defaults to `int' in
>>>>>>>>
>>>>>>>> declaration of
>>>>>>>>
>>>>>>>>>>> `interpreters_lock'
>>>>>>>>>>> mod_python.c:34: warning: data definition has no type or
>>>>>>>>
>>>>>>>> storage class
>>>>>>>>
>>>>>>>>>>> mod_python.c: In function `get_interpreter':
>>>>>>>>>>> mod_python.c:131: warning: implicit declaration of
>>>>>>>>>>> function
>>>>>>>>>>> `apr_thread_mutex_lock'
>>>>>>>>>>> mod_python.c:161: warning: implicit declaration of
>>>>>>>>>>> function
>>>>>>>>>>> `apr_thread_mutex_unlock'
>>>>>>>>>>> mod_python.c: In function `python_init':
>>>>>>>>>>> mod_python.c:517: warning: implicit declaration of
>>>>>>>>>>> function
>>>>>>>>>>> `apr_thread_mutex_create'
>>>>>>>>>>> mod_python.c:517: error: `APR_THREAD_MUTEX_UNNESTED'
>>>>>>>>
>>>>>>>> undeclared (first
>>>>>>>>
>>>>>>>>>>> use in this function)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Diff output
>>>>>>>>>>> -----------
>>>>>>>>>>> I've only copied the diff chunks which correspond to the
>>>>>>>>
>>>>>>>> complier
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> errors
>>>>>>>>>>
>>>>>>>>>>> mentioned above.
>>>>>>>>>>>
>>>>>>>>>>> --- mod_python-3.1.4/src/mod_python.c Sat Jan 29
>>>>>>>>>>> 13:25:28
>>>>>>>>
>>>>>>>> 2005
>>>>>>>>
>>>>>>>>>>> +++ mod_python-3.2.1b/src/mod_python.c Tue Sep 6
>>>>>>>>>>> 17:11:03
>>>>>>>>
>>>>>>>> 2005
>>>>>>>>
>>>>>>>>>>> @@ -31,6 +31,8 @@
>>>>>>>>>>> * (In a Python dictionary) */
>>>>>>>>>>> static PyObject * interpreters = NULL;
>>>>>>>>>>>
>>>>>>>>>>> +static apr_thread_mutex_t* interpreters_lock = 0;
>>>>>>>>>>> +
>>>>>>>>>>> apr_pool_t *child_init_pool = NULL;
>>>>>>>>>>>
>>>>>>>>>>> ... snip ...
>>>>>>>>>>>
>>>>>>>>>>> @@ -124,11 +128,15 @@
>>>>>>>>>>> name = MAIN_INTERPRETER;
>>>>>>>>>>>
>>>>>>>>>>> #ifdef WITH_THREAD
>>>>>>>>>>> + apr_thread_mutex_lock(interpreters_lock);
>>>>>>>>>>> PyEval_AcquireLock();
>>>>>>>>>>> #endif
>>>>>>>>>>>
>>>>>>>>>>> ... snip ...
>>>>>>>>>>>
>>>>>>>>>>> @@ -149,6 +158,7 @@
>>>>>>>>>>>
>>>>>>>>>>> #ifdef WITH_THREAD
>>>>>>>>>>> PyEval_ReleaseLock();
>>>>>>>>>>> + apr_thread_mutex_unlock(interpreters_lock);
>>>>>>>>>>> #endif
>>>>>>>>>>>
>>>>>>>>>>> ... snip ...
>>>>>>>>>>>
>>>>>>>>>>> @@ -490,13 +506,15 @@
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> /* initialize global Python interpreter if necessary */
>>>>>>>>>>> - if (! Py_IsInitialized())
>>>>>>>>>>> + if (initialized == 0 || !Py_IsInitialized())
>>>>>>>>>>> {
>>>>>>>>>>> -
>>>>>>>>>>> + initialized = 1;
>>>>>>>>>>> +
>>>>>>>>>>> /* initialze the interpreter */
>>>>>>>>>>> Py_Initialize();
>>>>>>>>>>>
>>>>>>>>>>> #ifdef WITH_THREAD
>>>>>>>>>>> +
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>> apr_thread_mutex_create(&interpreters_lock,APR_THREAD_MUTEX_UNNESTED,p);
>>>>
>>>>>>>>>>
>>>>>>>>>>> /* create and acquire the interpreter lock */
>>>>>>>>>>> PyEval_InitThreads();
>>>>>>>>>>> #endif
>>>>>>>>>>>
>>>>>>>>>>> So it would seem that the code causing the compile
>>>>>>>>>>> problems
>>>>>>>>
>>>>>>>> is new
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> for 3.2.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I also notice that in apr_arch_thread_mutex.h the
>>>>>>>>>>> typedef
>>>>>>>>
>>>>>>>> for
>>>>>>>>
>>>>>>>>>>> apr_thread_mutex_t is wrapped by #if APR_HAS_THREADS /
>>>>>>>>
>>>>>>>> #endif.
>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Looking at the apache source in
>>>>>>>>
>>>>>>>> srclib/apr/locks/unix/thread_mutex.c,
>>>>>>>>
>>>>>>>>>>> everything is also enclosed by #if APR_HAS_THREADS /
>>>>>>>>>>> #endif.
>>>>>>>>>>> eg, apr_thread_mutex_create, apr_thread_mutex_lock and
>>>>>>>>>>> apr_thread_mutex_unlock.
>>>>>>>>>>>
>>>>>>>>>>> Hopefully this will give someone a clue as to what may
>>>>>>>>>>> be
>>>>>>>>
>>>>>>>> going on
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> here
>>>>>>>>>>
>>>>>>>>>>> with FreeBSD.
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Jim
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
