Re: MODPYTHON-195
Hi Jeff
We use Apache on Windows too, but we haven't spent time tracking down
these leaks, so I'm really glad you're doing it :-)
David
Jeff Robbins wrote:
Graham,
Your version works well.
Thanks,
Jeff
P.S.
I know you're not testing on windows, but I found another leak in
mpm_winnt.c. I'm reporting it on jira. Am I the only guy using
apache on windows???
- Original Message - From: "Jeff Robbins" <[EMAIL PROTECTED]>
To: "Graham Dumpleton" <[EMAIL PROTECTED]>
Cc:
Sent: Wednesday, November 08, 2006 06:07
Subject: Re: MODPYTHON-195
Graham,
You placed the fix in a better spot than I did. I had placed it
after the apr_pool_userdata stuff due to not understanding what that
was all about. It makes better sense to simply test and exit the
python_init() routine as quickly as possible in the Win32 parent
process. Your comment is good, too. I will test your version and
confirm.
Thanks!
Jeff
- Original Message - From: "Graham Dumpleton"
<[EMAIL PROTECTED]>
To: "Jeff Robbins" <[EMAIL PROTECTED]>
Cc: "python-dev list"
Sent: Wednesday, November 08, 2006 05:38
Subject: Re: MODPYTHON-195
On 07/11/2006, at 10:51 PM, Jeff Robbins wrote:
Graham,
The problem on Win32 is that (I believe) we never want to
initialize Python in the persistent parent process. All the web
action is in the child process which is long-lived and it is this
child process that maintains the thread pool which services web
requests.
FWIW, in UNIX the initialisation of Python in the parent process is
a good thing
as it means it is only done once no matter how many child processes
there
are. This is because child processes are created as a fork of the
parent process
and so they inherit the already initialised Python interpreter,
thereby meaning
initialisation of the child process is quicker.
Since Win32 doesn't have an equivalent of fork, when the child process
is created the full Python initialisation is done anyway. Thus
avoiding the
initialisation of Python in the parent is probably reasonable.
The parent process as far as I can tell sits there to support
restarting the child process and support the Win32 Service Control
Manager (SCM) which has a protocol for how a process must respond
to certain messages in order to be a service on Win32.
I do not know how to use flags alone to distinguish the two
processes. The code I put in is not trying to restrict a call to
python_init() to only happen once in the parent process. It is
preventing python_init() from initializing Python in the parent
process.
I hope this clarifies things somewhat.
I want to note here that mpm_winnt.c line 1108 looks like this:
/* AP_PARENT_PID is only valid in the child */
pid = getenv("AP_PARENT_PID");
if (pid)
{
/* This is the child */
This environmental is how it knows to run certain code only in the
child process.
In summary,
if we do not want to run python_init() in the special Win32 parent
process, we need a way to distinguish this parent process from the
child process in which we DO want to run python_init(). The code
which maintains this dual process architecture (undoubtedly in
support of the Win32 service architecture) uses an environmental
that it purposefull creates and injects into the child process
"AP_PARENT_PID". I don't see how we can do better than use this
same distinguishing characterisic to know not to run python_init()
in the parent process.
As it stands I just may have to take you word on this as I don't
have first hand
access to Win32 platform (and don't want to) to experiment. The
AP_PARENT_PID
environment variable is at least present in all Apache 2.0 and 2.2
versions that
we support, so at least okay for a while if we rely on that. In the
future, if Apache
changes this, we will just need to accommodate any new/official way
of determining
it.
Anyway, is the following what you are expecting and is it placed
where you expect
it within the python_init() function?
static int initialized = 0;
#ifdef WIN32
/* No need to run python_init() in Win32 parent processes as
* the lack of fork on Win32 means we get no benefit as far as
* inheriting a preinitialized Python interpreter. Further,
* upon a restart on Win32 platform the python_init() function
* will be called again in the parent process but without some
* resources allocated by the previous call having being
* released properly, resulting in memory and Win32 resource
* leaks.
*/
if (!getenv("AP_PARENT_PID"))
return OK;
#endif /* WIN32 */
apr_pool_userdata_get(&data, userdata_key, s->process->pool);
if (!data) {
apr_pool_userdata_set((const void *)1, userdata_key,
apr_pool_cleanup_null, s->process->pool);
return OK;
}
Graham
Re: MODPYTHON-195
Graham,
Your version works well.
Thanks,
Jeff
P.S.
I know you're not testing on windows, but I found another leak in
mpm_winnt.c. I'm reporting it on jira. Am I the only guy using apache on
windows???
- Original Message -
From: "Jeff Robbins" <[EMAIL PROTECTED]>
To: "Graham Dumpleton" <[EMAIL PROTECTED]>
Cc:
Sent: Wednesday, November 08, 2006 06:07
Subject: Re: MODPYTHON-195
Graham,
You placed the fix in a better spot than I did. I had placed it after the
apr_pool_userdata stuff due to not understanding what that was all about.
It makes better sense to simply test and exit the python_init() routine as
quickly as possible in the Win32 parent process. Your comment is good,
too. I will test your version and confirm.
Thanks!
Jeff
- Original Message -
From: "Graham Dumpleton" <[EMAIL PROTECTED]>
To: "Jeff Robbins" <[EMAIL PROTECTED]>
Cc: "python-dev list"
Sent: Wednesday, November 08, 2006 05:38
Subject: Re: MODPYTHON-195
On 07/11/2006, at 10:51 PM, Jeff Robbins wrote:
Graham,
The problem on Win32 is that (I believe) we never want to initialize
Python in the persistent parent process. All the web action is in the
child process which is long-lived and it is this child process that
maintains the thread pool which services web requests.
FWIW, in UNIX the initialisation of Python in the parent process is a
good thing
as it means it is only done once no matter how many child processes
there
are. This is because child processes are created as a fork of the parent
process
and so they inherit the already initialised Python interpreter, thereby
meaning
initialisation of the child process is quicker.
Since Win32 doesn't have an equivalent of fork, when the child process
is created the full Python initialisation is done anyway. Thus avoiding
the
initialisation of Python in the parent is probably reasonable.
The parent process as far as I can tell sits there to support
restarting the child process and support the Win32 Service Control
Manager (SCM) which has a protocol for how a process must respond to
certain messages in order to be a service on Win32.
I do not know how to use flags alone to distinguish the two processes.
The code I put in is not trying to restrict a call to python_init() to
only happen once in the parent process. It is preventing python_init()
from initializing Python in the parent process.
I hope this clarifies things somewhat.
I want to note here that mpm_winnt.c line 1108 looks like this:
/* AP_PARENT_PID is only valid in the child */
pid = getenv("AP_PARENT_PID");
if (pid)
{
/* This is the child */
This environmental is how it knows to run certain code only in the
child process.
In summary,
if we do not want to run python_init() in the special Win32 parent
process, we need a way to distinguish this parent process from the
child process in which we DO want to run python_init(). The code
which maintains this dual process architecture (undoubtedly in support
of the Win32 service architecture) uses an environmental that it
purposefull creates and injects into the child process "AP_PARENT_PID".
I don't see how we can do better than use this same distinguishing
characterisic to know not to run python_init() in the parent process.
As it stands I just may have to take you word on this as I don't have
first hand
access to Win32 platform (and don't want to) to experiment. The
AP_PARENT_PID
environment variable is at least present in all Apache 2.0 and 2.2
versions that
we support, so at least okay for a while if we rely on that. In the
future, if Apache
changes this, we will just need to accommodate any new/official way of
determining
it.
Anyway, is the following what you are expecting and is it placed where
you expect
it within the python_init() function?
static int initialized = 0;
#ifdef WIN32
/* No need to run python_init() in Win32 parent processes as
* the lack of fork on Win32 means we get no benefit as far as
* inheriting a preinitialized Python interpreter. Further,
* upon a restart on Win32 platform the python_init() function
* will be called again in the parent process but without some
* resources allocated by the previous call having being
* released properly, resulting in memory and Win32 resource
* leaks.
*/
if (!getenv("AP_PARENT_PID"))
return OK;
#endif /* WIN32 */
apr_pool_userdata_get(&data, userdata_key, s->process->pool);
if (!data) {
apr_pool_userdata_set((const void *)1, userdata_key,
apr_pool_cleanup_null, s->process->pool);
return OK;
}
Graham
Re: MODPYTHON-195
Graham,
You placed the fix in a better spot than I did. I had placed it after the
apr_pool_userdata stuff due to not understanding what that was all about.
It makes better sense to simply test and exit the python_init() routine as
quickly as possible in the Win32 parent process. Your comment is good, too.
I will test your version and confirm.
Thanks!
Jeff
- Original Message -
From: "Graham Dumpleton" <[EMAIL PROTECTED]>
To: "Jeff Robbins" <[EMAIL PROTECTED]>
Cc: "python-dev list"
Sent: Wednesday, November 08, 2006 05:38
Subject: Re: MODPYTHON-195
On 07/11/2006, at 10:51 PM, Jeff Robbins wrote:
Graham,
The problem on Win32 is that (I believe) we never want to initialize
Python in the persistent parent process. All the web action is in the
child process which is long-lived and it is this child process that
maintains the thread pool which services web requests.
FWIW, in UNIX the initialisation of Python in the parent process is a
good thing
as it means it is only done once no matter how many child processes there
are. This is because child processes are created as a fork of the parent
process
and so they inherit the already initialised Python interpreter, thereby
meaning
initialisation of the child process is quicker.
Since Win32 doesn't have an equivalent of fork, when the child process
is created the full Python initialisation is done anyway. Thus avoiding
the
initialisation of Python in the parent is probably reasonable.
The parent process as far as I can tell sits there to support restarting
the child process and support the Win32 Service Control Manager (SCM)
which has a protocol for how a process must respond to certain messages
in order to be a service on Win32.
I do not know how to use flags alone to distinguish the two processes.
The code I put in is not trying to restrict a call to python_init() to
only happen once in the parent process. It is preventing python_init()
from initializing Python in the parent process.
I hope this clarifies things somewhat.
I want to note here that mpm_winnt.c line 1108 looks like this:
/* AP_PARENT_PID is only valid in the child */
pid = getenv("AP_PARENT_PID");
if (pid)
{
/* This is the child */
This environmental is how it knows to run certain code only in the child
process.
In summary,
if we do not want to run python_init() in the special Win32 parent
process, we need a way to distinguish this parent process from the child
process in which we DO want to run python_init(). The code which
maintains this dual process architecture (undoubtedly in support of the
Win32 service architecture) uses an environmental that it purposefull
creates and injects into the child process "AP_PARENT_PID". I don't see
how we can do better than use this same distinguishing characterisic to
know not to run python_init() in the parent process.
As it stands I just may have to take you word on this as I don't have
first hand
access to Win32 platform (and don't want to) to experiment. The
AP_PARENT_PID
environment variable is at least present in all Apache 2.0 and 2.2
versions that
we support, so at least okay for a while if we rely on that. In the
future, if Apache
changes this, we will just need to accommodate any new/official way of
determining
it.
Anyway, is the following what you are expecting and is it placed where
you expect
it within the python_init() function?
static int initialized = 0;
#ifdef WIN32
/* No need to run python_init() in Win32 parent processes as
* the lack of fork on Win32 means we get no benefit as far as
* inheriting a preinitialized Python interpreter. Further,
* upon a restart on Win32 platform the python_init() function
* will be called again in the parent process but without some
* resources allocated by the previous call having being
* released properly, resulting in memory and Win32 resource
* leaks.
*/
if (!getenv("AP_PARENT_PID"))
return OK;
#endif /* WIN32 */
apr_pool_userdata_get(&data, userdata_key, s->process->pool);
if (!data) {
apr_pool_userdata_set((const void *)1, userdata_key,
apr_pool_cleanup_null, s->process->pool);
return OK;
}
Graham
Re: MODPYTHON-195
Graham,
The problem on Win32 is that (I believe) we never want to initialize Python
in the persistent parent process. All the web action is in the child
process which is long-lived and it is this child process that maintains the
thread pool which services web requests.
The parent process as far as I can tell sits there to support restarting the
child process and support the Win32 Service Control Manager (SCM) which has
a protocol for how a process must respond to certain messages in order to be
a service on Win32.
I do not know how to use flags alone to distinguish the two processes. The
code I put in is not trying to restrict a call to python_init() to only
happen once in the parent process. It is preventing python_init() from
initializing Python in the parent process.
I hope this clarifies things somewhat.
I want to note here that mpm_winnt.c line 1108 looks like this:
/* AP_PARENT_PID is only valid in the child */
pid = getenv("AP_PARENT_PID");
if (pid)
{
/* This is the child */
This environmental is how it knows to run certain code only in the child
process.
In summary,
if we do not want to run python_init() in the special Win32 parent process,
we need a way to distinguish this parent process from the child process in
which we DO want to run python_init(). The code which maintains this dual
process architecture (undoubtedly in support of the Win32 service
architecture) uses an environmental that it purposefull creates and injects
into the child process "AP_PARENT_PID". I don't see how we can do better
than use this same distinguishing characterisic to know not to run
python_init() in the parent process.
- Original Message -
From: "Graham Dumpleton" <[EMAIL PROTECTED]>
To: "Jeff Robbins" <[EMAIL PROTECTED]>
Cc: "python-dev list"
Sent: Tuesday, November 07, 2006 06:02
Subject: Re: MODPYTHON-195
On 04/11/2006, at 12:34 PM, Jeff Robbins wrote:
Graham,
I haven't had any new ideas about this problem. It is clear that on
Windows, mod_python is initialized both in a parent process and more
usefully in the child process that spins up the threads that service
client requests. The parent process is long-lived and the standard hack
to wait for the second call to the ap_hook_post_config is useless
because each "restart" of apache is yet another call (third, fourth,
fifth, etc...) and each time there's a leak of one handle.
The fix I tested seems reasonable. I know it is dependent on
mpm_winnt.c, but, after all, that file is the file responsible for the
dual process architecture on windows to begin with. And the fix has an
#ifdef win32 so it won't hurt linux users.
I'd like you to consider folding it in. I think it is better than
having a leak (along with spurious python initialization) on windows.
Jeff, can you see if you can come up with a test based on 'initialized'
and
'child_init_pool' as I note in:
http://issues.apache.org/jira/browse/MODPYTHON-195
If it is only in the parent process you need to skip subsequent calls,
perhaps:
if (child_init_pool == 0 && initialized != 0)
return OK;
Will have to think about how this may screw up old versions of Mac OS X
though which is why initialized was added in the first place.
You might include in your debug a call to Py_IsInitialized() so it can be
determined if Python thinks it is already initialised. Since your fiddle
is
working, I'd say it probably is.
Also see if main_server is set and not zero as well and whether its value
is different to 's' passed as argument to function. Whether it is the
same
or not may dictate where in function the check to bail out of function
needs
to be. It may have to go just before the global config and mutexes are
created.
Graham
Re: MODPYTHON-195
Graham,
The problem on Win32 is that (I believe) we never want to initialize Python
in the persistent parent process. All the web action is in the child
process which is long-lived and it is this child process that maintains the
thread pool which services web requests.
The parent process as far as I can tell sits there to support restarting the
child process and support the Win32 Service Control Manager (SCM) which has
a protocol for how a process must respond to certain messages in order to be
a service on Win32.
I do not know how to use flags alone to distinguish the two processes. The
code I put in is not trying to restrict a call to python_init() to only
happen once in the parent process. It is preventing python_init() from
initializing Python in the parent process.
I hope this clarifies things somewhat.
I want to note here that mpm_winnt.c line 1108 looks like this:
/* AP_PARENT_PID is only valid in the child */
pid = getenv("AP_PARENT_PID");
if (pid)
{
/* This is the child */
This environmental is how it knows to run certain code only in the child
process.
In summary,
if we do not want to run python_init() in the special Win32 parent process,
we need a way to distinguish this parent process from the child process in
which we DO want to run python_init(). The code which maintains this dual
process architecture (undoubtedly in support of the Win32 service
architecture) uses an environmental that it purposefull creates and injects
into the child process "AP_PARENT_PID". I don't see how we can do better
than use this same distinguishing characterisic to know not to run
python_init() in the parent process.
- Original Message -
From: "Graham Dumpleton" <[EMAIL PROTECTED]>
To: "Jeff Robbins" <[EMAIL PROTECTED]>
Cc: "python-dev list"
Sent: Tuesday, November 07, 2006 06:02
Subject: Re: MODPYTHON-195
On 04/11/2006, at 12:34 PM, Jeff Robbins wrote:
Graham,
I haven't had any new ideas about this problem. It is clear that on
Windows, mod_python is initialized both in a parent process and more
usefully in the child process that spins up the threads that service
client requests. The parent process is long-lived and the standard hack
to wait for the second call to the ap_hook_post_config is useless
because each "restart" of apache is yet another call (third, fourth,
fifth, etc...) and each time there's a leak of one handle.
The fix I tested seems reasonable. I know it is dependent on
mpm_winnt.c, but, after all, that file is the file responsible for the
dual process architecture on windows to begin with. And the fix has an
#ifdef win32 so it won't hurt linux users.
I'd like you to consider folding it in. I think it is better than
having a leak (along with spurious python initialization) on windows.
Jeff, can you see if you can come up with a test based on 'initialized'
and
'child_init_pool' as I note in:
http://issues.apache.org/jira/browse/MODPYTHON-195
If it is only in the parent process you need to skip subsequent calls,
perhaps:
if (child_init_pool == 0 && initialized != 0)
return OK;
Will have to think about how this may screw up old versions of Mac OS X
though which is why initialized was added in the first place.
You might include in your debug a call to Py_IsInitialized() so it can be
determined if Python thinks it is already initialised. Since your fiddle
is
working, I'd say it probably is.
Also see if main_server is set and not zero as well and whether its value
is different to 's' passed as argument to function. Whether it is the
same
or not may dictate where in function the check to bail out of function
needs
to be. It may have to go just before the global config and mutexes are
created.
Graham
Re: MODPYTHON-195
On 04/11/2006, at 12:34 PM, Jeff Robbins wrote: Graham, I haven't had any new ideas about this problem. It is clear that on Windows, mod_python is initialized both in a parent process and more usefully in the child process that spins up the threads that service client requests. The parent process is long-lived and the standard hack to wait for the second call to the ap_hook_post_config is useless because each "restart" of apache is yet another call (third, fourth, fifth, etc...) and each time there's a leak of one handle. The fix I tested seems reasonable. I know it is dependent on mpm_winnt.c, but, after all, that file is the file responsible for the dual process architecture on windows to begin with. And the fix has an #ifdef win32 so it won't hurt linux users. I'd like you to consider folding it in. I think it is better than having a leak (along with spurious python initialization) on windows. Jeff, can you see if you can come up with a test based on 'initialized' and 'child_init_pool' as I note in: http://issues.apache.org/jira/browse/MODPYTHON-195 If it is only in the parent process you need to skip subsequent calls, perhaps: if (child_init_pool == 0 && initialized != 0) return OK; Will have to think about how this may screw up old versions of Mac OS X though which is why initialized was added in the first place. You might include in your debug a call to Py_IsInitialized() so it can be determined if Python thinks it is already initialised. Since your fiddle is working, I'd say it probably is. Also see if main_server is set and not zero as well and whether its value is different to 's' passed as argument to function. Whether it is the same or not may dictate where in function the check to bail out of function needs to be. It may have to go just before the global config and mutexes are created. Graham
