perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: [PATCH] libperl leaks a THREAD_KEY each time it is reloaded
Date Wed, 12 Jan 2005 01:32:13 GMT
Gurusamy Sarathy wrote:
> On Tue, 11 Jan 2005 14:55:12 EST, Stas Bekman wrote:
> 
>>>The '#ifdef ALLOC_THREAD_KEY' is wrong AFAICT; this macro will always
>>>be defined.  Testing PL_thr_key as a boolean is outside of the spec of
>>>what pthreads define. PL_curinterp will be set if the key has
>>>initialized.  That's a better test.
>>>
>>>Does things not work if you just drop in the "perl_fini" destructor I
>>>provided in mod_perl.c?
>>
>>It works too. The latest version is:
>>
>>Index: src/modules/perl/mod_perl.c
>>===================================================================
>>--- src/modules/perl/mod_perl.c (revision 124805)
>>+++ src/modules/perl/mod_perl.c (working copy)
>>@@ -573,6 +573,17 @@
>>     MP_threads_started = 0;
>>     MP_post_post_config_phase = 0;
>>
>>+    /* with USE_ITHREADS perl leaks pthread_key_t on every
>>+     * perl_destruct, which becomes a problem restarts: if the OS
> 
> 
> This is not a leak, that's how it is supposed to work.

How can you say that it's not a leak, if perl allocates the key again and 
again. mod_perl doesn't do anything forbidden, just calls 
perl_(alloc|parse|destruct|free). It's just that until now the problem was 
hidden. Any embed app that has to restart will have exactly the same problem.

> A pthread_key_t gets allocated for the lifetime of a process
> or for the duration a shared library remains loaded.  It is not
> something that is local to a single perl interpreter as the
> comment above seems to imply.

Jan and Gisle have already explained that to me :) It was just an old 
version of the comment I forgot to adjust.

>>+     * limit is 1024, 1024 restarts later things will start
>>+     * crashing */
>>+    /* XXX: this is a workaround for the bug in perl, once and if it's
>>+     * fixed we need to disable it for the versions that have it
>>+     * fixed */
>>+    if (PL_curinterp) {
>>+        FREE_THREAD_KEY;
>>+    }
>>+
>>     MP_TRACE_i(MP_FUNC, "mod_perl sys term\n");
>>
>>     modperl_env_unload();
> 
> 
> This code looks dangerous for embedded applications that might use
> perl interpreters other than the ones under mod_perl's control.
> 
> Perhaps you don't fully understand the implications of deallocating
> the thread key?  From the pthread_key_delete() man page:
> 
> :    For an application to know that it is safe to delete a key, it  has  to
> :    know  that  all  the threads that might potentially ever use the key do
> :    not attempt to use it again. For example, it could know this if all the
> :    client  threads have called a cleanup procedure declaring that they are
> :    through with the module that is being shut down, perhaps by  setting  a
> :    reference count to zero.
> 
> IOW, some kind of reference counting is needed before you can
> safely call pthread_key_delete() because some other thread that
> is not under mod_perl's control could have created its own
> perl interpreter and still be in the process of using it.

I agree, but I just try to provide a workaround for the bug in perl. I 
can't do any ref counting on behalf of perl.

> The real problem in your case seems to be that pthread_key_create()
> is repeatedly being called, when it shouldn't be.  The only reasons
> I see for this are:
> 
> 1. PL_curinterp getting set to NULL at some point by mod_perl, or
> 
> 2. Unload/reload of the shared object that contains libperl.a
>    which causes PL_curinterp to be reset to NULL.
> 
> To solve the problem for the first case, remove the code in mod_perl
> that sets PL_curinterp to NULL (because you're not supposed to do
> that).  In the second case, Gisle's patch is in fact the right
> approach to fix the problem.

No, no, please disregard my test program I've posted originally. It's no 
longer relevant. mod_perl does *not* sets PL_curinterp to NULL and 
pthread_key_create() is called only once.

> An even better approach might be to make Perl use pthread_once()
> to allocate its perl_key instead of relying on PL_curinterp being
> a never-NULL variable.  Doing this might actually make Gisle's
> fix unnecessary.  This will also probably make it unnecessary to
> call pthread_key_delete() anywhere.

Sounds like an excellent idea to me.

We would still need to provide some sort of workaround. May be we will 
just redefine the macro called from perl_alloc. I'll have to see how 
Nicholas solves it first.

So Nicholas, the ball is on your ground.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message