[5.8.1] getppid is not maintained if not forked by perl
Stas Bekman
in 5.8.0. When ithreads are enabled the new logic updates PL_ppid in pp_fork,
which now does:
PP(pp_fork)
{
...
#ifdef THREADS_HAVE_PIDS
PL_ppid = (IV)getppid();
#endif
...
So if an application running embedded perl wasn't forked by perl's pp_fork,
it'll have this problem. Under mod_perl 2, we get getppid reporting 1 from the
forked process, when pstree clearly shows that this is not the case.
I have a workaround for mp2:
#if PERL_REVISION == 5 && PERL_VERSION == 8 && PERL_SUBVERSION == 1
{
#ifdef THREADS_HAVE_PIDS
#ifdef USE_ITHREADS
MP_dSCFG(s);
dTHXa(scfg->mip->parent->perl);
PL_ppid = (IV)getppid();
#endif
#endif
}
#endif
which can be run at the child_init phase, immediately after a new process is
forked. But may be it's something that need to be fixed in perl, as it worked
just fine with 5.8.0, and is probably an inappropriate change in behavior for
a maintenance release? If not please let me know, so that I'll change the
workaround's ifdef to match 5.8.1+.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:st...@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Rafael Garcia-Suarez
> It looks like 5.8.1 started to cache ppids in PL_ppid, which wasn't the case
> in 5.8.0. When ithreads are enabled the new logic updates PL_ppid in pp_fork,
> which now does:
>
> PP(pp_fork)
> {
> ...
> #ifdef THREADS_HAVE_PIDS
> PL_ppid = (IV)getppid();
> #endif
THREADS_HAVE_PIDS is only defined on Linux.
The relevant section of the docs is in perlvar/$$ :
Note for Linux users: on Linux, the C functions C<getpid()> and
C<getppid()> return different values from different threads. In order to
be portable, this behavior is not reflected by C<$$>, whose value remains
consistent across threads. If you want to call the underlying C<getpid()>,
you may use the CPAN module C<Linux::Pid>.
> So if an application running embedded perl wasn't forked by perl's pp_fork,
> it'll have this problem. Under mod_perl 2, we get getppid reporting 1 from the
> forked process, when pstree clearly shows that this is not the case.
Yes. I didn't thought about this problem.
> I have a workaround for mp2:
>
> #if PERL_REVISION == 5 && PERL_VERSION == 8 && PERL_SUBVERSION == 1
> {
> #ifdef THREADS_HAVE_PIDS
> #ifdef USE_ITHREADS
> MP_dSCFG(s);
> dTHXa(scfg->mip->parent->perl);
> PL_ppid = (IV)getppid();
> #endif
> #endif
> }
> #endif
>
> which can be run at the child_init phase, immediately after a new process is
> forked. But may be it's something that need to be fixed in perl, as it worked
> just fine with 5.8.0, and is probably an inappropriate change in behavior for
> a maintenance release? If not please let me know, so that I'll change the
> workaround's ifdef to match 5.8.1+.
Currently PL_ppid is initialized in S_init_postdump_symbols.
I imagine we could initialize or reinitialize it later at a time
more suitable if there is some routine to be called back at fork
time.
Perhaps should I add a note in perlembed.pod about this problem ?
Stas Bekman
> Stas Bekman wrote:
>
>>It looks like 5.8.1 started to cache ppids in PL_ppid, which wasn't the case
>>in 5.8.0. When ithreads are enabled the new logic updates PL_ppid in pp_fork,
>>which now does:
>>
>>PP(pp_fork)
>>{
>>...
>>#ifdef THREADS_HAVE_PIDS
>> PL_ppid = (IV)getppid();
>>#endif
>
>
> THREADS_HAVE_PIDS is only defined on Linux.
> The relevant section of the docs is in perlvar/$$ :
>
> Note for Linux users: on Linux, the C functions C<getpid()> and
> C<getppid()> return different values from different threads. In order to
> be portable, this behavior is not reflected by C<$$>, whose value remains
> consistent across threads. If you want to call the underlying C<getpid()>,
> you may use the CPAN module C<Linux::Pid>.
Yes, but we aren't talking threads here. This behavior is true under
mp2/prefork mpm where we have no threads, but processes. We run in the
threads-friendly environment, so you can spawn threads if you want to (e.g. if
you want to use more than one perl interpreter under the same process).
>>So if an application running embedded perl wasn't forked by perl's pp_fork,
>>it'll have this problem. Under mod_perl 2, we get getppid reporting 1 from the
>>forked process, when pstree clearly shows that this is not the case.
>
>
> Yes. I didn't thought about this problem.
>
>>I have a workaround for mp2:
>>
>>#if PERL_REVISION == 5 && PERL_VERSION == 8 && PERL_SUBVERSION == 1
>> {
>>#ifdef THREADS_HAVE_PIDS
>>#ifdef USE_ITHREADS
>> MP_dSCFG(s);
>> dTHXa(scfg->mip->parent->perl);
>> PL_ppid = (IV)getppid();
>>#endif
>>#endif
>> }
>>#endif
>>
>>which can be run at the child_init phase, immediately after a new process is
>>forked. But may be it's something that need to be fixed in perl, as it worked
>>just fine with 5.8.0, and is probably an inappropriate change in behavior for
>>a maintenance release? If not please let me know, so that I'll change the
>>workaround's ifdef to match 5.8.1+.
>
>
> Currently PL_ppid is initialized in S_init_postdump_symbols.
> I imagine we could initialize or reinitialize it later at a time
> more suitable if there is some routine to be called back at fork
> time.
I don't think this should be a responsibility of the application, it's quite
possible that that the application embedding perl has no callback/event to
know when to call this special initialization.
I think the really proper algorithm is to leave the current behavior
only if perl was the one who did the fork (which will probably require yet
another PL_ flag. And if that flag is not set the pp_getppid logic should be
changed to cache that value and raise that flag. Something like this:
PP(pp_fork)
{
...
#ifdef THREADS_HAVE_PIDS
PL_ppid = (IV)getppid();
PL_ppid_cached = 1;
#endif
PP(pp_getppid)
{
...
{
#ifdef HAS_GETPPID
dSP; dTARGET;
# ifdef THREADS_HAVE_PIDS
if (!PL_ppid_cached) {
PL_ppid = (IV)getppid();
PL_ppid_cached = 1;
}
XPUSHi( PL_ppid );
# else
XPUSHi( getppid() );
# endif
RETURN;
#else
DIE(aTHX_ PL_no_func, "getppid");
#endif
}
of course needing to init PL_ppid_cached to 0 when created and make sure that
threads init it to 0 as well, I suppose. this is untested, just an idea. Or
maybe initting PL_ppid to 0, can serve the same purpose as the newly suggested
flag PL_ppid_cached. So the logic would be then:
PP(pp_getppid)
{
...
{
#ifdef HAS_GETPPID
dSP; dTARGET;
# ifdef THREADS_HAVE_PIDS
if (!PL_ppid) {
PL_ppid = (IV)getppid();
}
XPUSHi( PL_ppid );
# else
...
without any changes to pp_fork
> Perhaps should I add a note in perlembed.pod about this problem ?
First of all, do you think this change (if left unmodified) is suitable for a
maintenance release?
Rafael Garcia-Suarez
> >
> > THREADS_HAVE_PIDS is only defined on Linux.
> > The relevant section of the docs is in perlvar/$$ :
> >
> > Note for Linux users: on Linux, the C functions C<getpid()> and
> > C<getppid()> return different values from different threads. In order to
> > be portable, this behavior is not reflected by C<$$>, whose value remains
> > consistent across threads. If you want to call the underlying C<getpid()>,
> > you may use the CPAN module C<Linux::Pid>.
>
> Yes, but we aren't talking threads here. This behavior is true under
> mp2/prefork mpm where we have no threads, but processes. We run in the
> threads-friendly environment, so you can spawn threads if you want to (e.g. if
> you want to use more than one perl interpreter under the same process).
I was providing some background on the rationale behind this change.
Basically the consensus at the time was that $$, getp?pid and
POSIX::getp?pid need to be portable, i.e. return the same process id from
different threads ; and consequently that Linux's non-standard behavior
of assigning differents PIDs to different threads had to be hidden.
> I think the really proper algorithm is to leave the current behavior
> only if perl was the one who did the fork (which will probably require yet
> another PL_ flag. And if that flag is not set the pp_getppid logic should be
> changed to cache that value and raise that flag. Something like this:
Yes. I'll take a look at this this week-end if nobody beats me to it.
> First of all, do you think this change (if left unmodified) is suitable for a
> maintenance release?
Well, the pid change is already out there in perl 5.8.1, so we need to
make it more friendly for applications that embed perl on Linux. I think
the fix should go both in blead and maint.
Stas Bekman
> Stas Bekman wrote:
>
>>>THREADS_HAVE_PIDS is only defined on Linux.
>>>The relevant section of the docs is in perlvar/$$ :
>>>
>>> Note for Linux users: on Linux, the C functions C<getpid()> and
>>> C<getppid()> return different values from different threads. In order to
>>> be portable, this behavior is not reflected by C<$$>, whose value remains
>>> consistent across threads. If you want to call the underlying C<getpid()>,
>>> you may use the CPAN module C<Linux::Pid>.
>>
>>Yes, but we aren't talking threads here. This behavior is true under
>>mp2/prefork mpm where we have no threads, but processes. We run in the
>>threads-friendly environment, so you can spawn threads if you want to (e.g. if
>>you want to use more than one perl interpreter under the same process).
>
>
> I was providing some background on the rationale behind this change.
> Basically the consensus at the time was that $$, getp?pid and
> POSIX::getp?pid need to be portable, i.e. return the same process id from
> different threads ; and consequently that Linux's non-standard behavior
> of assigning differents PIDs to different threads had to be hidden.
Understood. Thanks Rafael!
>>First of all, do you think this change (if left unmodified) is suitable for a
>>maintenance release?
>
>
> Well, the pid change is already out there in perl 5.8.1, so we need to
> make it more friendly for applications that embed perl on Linux. I think
> the fix should go both in blead and maint.
Seeing the rush of restoring some things as they were in 5.8.0 for 5.8.2, I
just thought that this could be one of the things that needs to be reverted as
well. Of course if the patch that we discussed does the trick, than it's fine.
I wish I had many more mp2 test written and getting to discover new problems,
just because there wasn't a test covering it ;) But our test suite is growing
like crazy, so it'll be much easier to test new perl generations with it.
Rafael Garcia-Suarez
> >
> > Well, the pid change is already out there in perl 5.8.1, so we need to
> > make it more friendly for applications that embed perl on Linux. I think
> > the fix should go both in blead and maint.
>
> Seeing the rush of restoring some things as they were in 5.8.0 for 5.8.2, I
> just thought that this could be one of the things that needs to be reverted as
> well. Of course if the patch that we discussed does the trick, than it's fine.
Haven't we another similar (but more general) problem with $$ ?
While the PPID is cached in PL_ppid on Linux and for perl >= 5.8.1 with
ithreads, the PID is cached, for all perl versions I know of, and on all
platforms, in $$ (which is a regular SV).
That means that if an application that embeds perl forks, $$ is
not going to be updated.
As far as I can tell this doesn't impact mod_perl 1, because of
mod_perl_init_ids() in perl_util.c (called at child init time).
I don't think this happens on mod_perl 2 with prefork MPM, but I'm not
sure why. Could someone try with mod_perl 2 / threaded or prefork MPM to
see if the problem on $$ exists ? (Sorry, lacking time to test this
myself now) I see that mod_perl 2 has a function modperl_perl_init_ids()
in modperl_perl.c. That would probably a good place to reinit PL_ppid
as well.
(I tend to think that the best way to handle this in the general case
is to provide from perl a public API to reset $$ and ppid.)
Nicholas Clark
> Seeing the rush of restoring some things as they were in 5.8.0 for 5.8.2, I
> just thought that this could be one of the things that needs to be reverted
> as well. Of course if the patch that we discussed does the trick, than it's
> fine.
rush? what rush?
Or have I misunderstood. Do you mean "rush" as in urgency in as much as
people are trying to get things done quickly to meet a perceived tight
release schedule (but the list of things is small)?
Or "rush" as in many things are all arriving wanting to be reverted?
The only thing that I was aware of driving the desire to have a 5.8.2
soon was the binary compatibility issue over hashes. If this is true,
then I'm minded to release a 5.8.2 with the bare minimum of changes, and
everything else can wait until 5.8.3, which is pencilled in for January
2004.
(I'm not aware of any other binary compatibility issues. They may exist,
but I am not aware of them, so please remind me of them, or tell me about
them for the first time, or forever hold your peace. [Other options may
be available :-)])
Nicholas Clark
Rafael Garcia-Suarez
> I don't think this happens on mod_perl 2 with prefork MPM, but I'm not
> sure why. Could someone try with mod_perl 2 / threaded or prefork MPM to
> see if the problem on $$ exists ? (Sorry, lacking time to test this
> myself now) I see that mod_perl 2 has a function modperl_perl_init_ids()
> in modperl_perl.c. That would probably a good place to reinit PL_ppid
> as well.
And indeed this (tested) patch against mod_perl 1.99_10 fixes it.
I haven't checked if the same problem occurs with mod_perl 1.
--- src/modules/perl/modperl_perl.c.orig Fri Oct 17 22:33:58 2003
+++ src/modules/perl/modperl_perl.c Sat Oct 18 00:53:46 2003
@@ -33,6 +33,9 @@ void modperl_perl_core_global_init(pTHX)
static void modperl_perl_ids_get(modperl_perl_ids_t *ids)
{
ids->pid = (I32)getpid();
+#ifdef THREADS_HAVE_PIDS
+ ids->ppid = (I32)getppid();
+#endif
#ifndef WIN32
ids->uid = getuid();
ids->euid = geteuid();
@@ -55,6 +58,9 @@ static void modperl_perl_init_ids(pTHX_
PL_euid = ids->euid;
PL_gid = ids->gid;
PL_egid = ids->egid;
+#endif
+#ifdef THREADS_HAVE_PIDS
+ PL_ppid = ids->ppid;
#endif
}
--- src/modules/perl/modperl_perl.h.orig Fri Oct 17 22:35:57 2003
+++ src/modules/perl/modperl_perl.h Sat Oct 18 00:53:57 2003
@@ -5,6 +5,9 @@
I32 pid;
Uid_t uid, euid;
Gid_t gid, egid;
+#ifdef THREADS_HAVE_PIDS
+ Uid_t ppid;
+#endif
} modperl_perl_ids_t;
void modperl_perl_core_global_init(pTHX);
End.
Ton Hospel
Rafael Garcia-Suarez <rgarci...@free.fr> writes:
> THREADS_HAVE_PIDS is only defined on Linux.
> The relevant section of the docs is in perlvar/$$ :
>
> Note for Linux users: on Linux, the C functions C<getpid()> and
> C<getppid()> return different values from different threads. In order to
> be portable, this behavior is not reflected by C<$$>, whose value remains
> consistent across threads. If you want to call the underlying C<getpid()>,
> you may use the CPAN module C<Linux::Pid>.
>
But notice that since 2.4 linux has the CLONE_THREAD flag to clone(2),
and getpid(2) return the thread group ID of the caller. This change was
explicitely done to make linux threads more POSIX like. So the hack should
only be relevant on older linuxes anyways (if thread creation is done
using CLONE_THREAD).
Stas Bekman
> Rafael Garcia-Suarez wrote:
>
>>I don't think this happens on mod_perl 2 with prefork MPM, but I'm not
>>sure why. Could someone try with mod_perl 2 / threaded or prefork MPM to
>>see if the problem on $$ exists ? (Sorry, lacking time to test this
>>myself now) I see that mod_perl 2 has a function modperl_perl_init_ids()
>>in modperl_perl.c. That would probably a good place to reinit PL_ppid
>>as well.
>
>
> And indeed this (tested) patch against mod_perl 1.99_10 fixes it.
Thanks Rafael. In the future please post patches against the current cvs if
possible, as I've already applied a similar fix, though yours is definitely
better. The cvs version already includes a test for this bug.
Though in your patch should we check for:
#if PERL_REVISION == 5 && PERL_VERSION == 8 && PERL_SUBVERSION == 1
and higher and otherwise do nothing? Or do you think that checking for
THREADS_HAVE_PIDS is enough? won't it be set as well in 5.6.x built with threads?
__________________________________________________________________
Stas Bekman
With your patch both threaded/prefork seem to do alright. The interesting
thing is that with worker (threaded) mpm the test prints:
# ppid 18673
ok 1
# pid 18675
ok 2
And the ps tree shows:
% pstree -p | grep httpd
|-httpd(18673)-+-httpd(18674)
| `-httpd(18675)---httpd(18676)-+-httpd(18678)
| |-httpd(18679)
| `-httpd(18680)
the last column are real httpd threads, the first is the real parent and the
second column are supposedly the procs containing the httpd threads. I'm not
sure what's the third column. It's like there is yet another process in the
middle. So I'm not sure whether the whole picture is correct.
Of course under preforked mpm it's simple:
|-httpd(23678)-+-httpd(23679)
| `-httpd(23682)
# ppid 23678
ok 1
# pid 23679
ok 2
Nicholas Clark
> Rafael Garcia-Suarez wrote:
> >And indeed this (tested) patch against mod_perl 1.99_10 fixes it.
>
> Thanks Rafael. In the future please post patches against the current cvs if
> possible, as I've already applied a similar fix, though yours is definitely
> better. The cvs version already includes a test for this bug.
On IRC Rafael said that he believes that his patch solves the problem
without needing changes to perl. Is this the consensus view?
If so, and if I get confirmation that the hash changes restore binary
compatibility with 5.8.0 (without affecting binary compatibility with
5.8.1), I see nothing holding us back from a release candidate for 5.8.2.
(Although there will be a snapshot first, hopefully this weekend)
Given that short time between 5.8.1 and now, you can infer how much
has changed in 5.8.2-tobe :-)
Normal service (on the new timetable) will be resumed soon. Expect
5.8.3 in January 2004, and 5.8.4 about 3 months later.
(or that's the plan. if people say "5.8.2 now is crazy" enough, then
expect 5.8.2 in January, and 5.8.3 ...)
Nicholas Clark
Stas Bekman
> On Sat, Oct 18, 2003 at 10:20:35AM -0700, Stas Bekman wrote:
>
>>Rafael Garcia-Suarez wrote:
>
>
>>>And indeed this (tested) patch against mod_perl 1.99_10 fixes it.
>>
>>Thanks Rafael. In the future please post patches against the current cvs if
>>possible, as I've already applied a similar fix, though yours is definitely
>>better. The cvs version already includes a test for this bug.
>
>
> On IRC Rafael said that he believes that his patch solves the problem
> without needing changes to perl. Is this the consensus view?
Rafael's patch solves the problem in mod_perl 2.0. The problem still exists
for any other application that embeds perl and does the fork on its own (not
using pp_fork). So 5.8.2 (same as 5.8.1) may break applications that ran OK
under 5.8.0. I've suggested a patch to pp_getppid that fixes this problem in perl.
Stas Bekman
Sorry Nick, I didn't imply any bad meaning by saying "rush". All I wanted to
say is that since 5.8.2 is a maint release, it could fix problems in 5.8.1
which didn't exist in 5.8.0. It'll take awhile before people will move to
5.8.1, so if 5.8.2 is released shortly, and it fixes incompatible changes
between 5.8.1 and 5.8.0, nobody will notice those incompatibilities, as people
will move directly to 5.8.2, bypassing 5.8.1.
Rafael Garcia-Suarez
>
> Though in your patch should we check for:
>
> #if PERL_REVISION == 5 && PERL_VERSION == 8 && PERL_SUBVERSION == 1
> and higher and otherwise do nothing? Or do you think that checking for
> THREADS_HAVE_PIDS is enough? won't it be set as well in 5.6.x built with threads?
THREADS_HAVE_PIDS is set in the Linux Configure hintfile.
For extra safety, now that you mention it, you should check for
PERL_VERSION == 8 && PERL_SUBVERSION >= 1 || PERL_VERSION >= 9.
(Because I integrated the hints from bleadperl to 5.6.2.)
Nicholas Clark
> Sorry Nick, I didn't imply any bad meaning by saying "rush". All I wanted
> to say is that since 5.8.2 is a maint release, it could fix problems in
> 5.8.1 which didn't exist in 5.8.0. It'll take awhile before people will
> move to 5.8.1, so if 5.8.2 is released shortly, and it fixes incompatible
> changes between 5.8.1 and 5.8.0, nobody will notice those
> incompatibilities, as people will move directly to 5.8.2, bypassing 5.8.1.
Aha. This is a good point, about jumping past 5.8.1, as it's why Chip is
arguing for a prompt 5.8.2 to address the hash issue.
But what are the other problems?
I'm not aware of a list, let alone a prioritised list.
Nicholas Clark
Rafael Garcia-Suarez
> Nicholas Clark wrote:
> > On IRC Rafael said that he believes that his patch solves the problem
> > without needing changes to perl. Is this the consensus view?
>
> Rafael's patch solves the problem in mod_perl 2.0. The problem still exists
> for any other application that embeds perl and does the fork on its own (not
> using pp_fork). So 5.8.2 (same as 5.8.1) may break applications that ran OK
> under 5.8.0. I've suggested a patch to pp_getppid that fixes this problem in perl.
On the other hand, the problem exists for $$ (there is no getpid function
in perl) in all perls. So all applications that embed perl and that fork need
at least to save and restore the PID (mod_perl 1 & 2 do this.) It's up to
Nicholas to see if he wants to restore the 5.8.0 behavior in 5.8.1 but I think
that PL_ppid will stay in 5.10.
I don't think your proposed patch works in the case where perl does a
getppid(), so it marks it as cached, then the embedding application does
a fork, and the PL_ppid_cached flag stays to "true".
(Oh, and sorry for my weak mod_perl patch-fu :) Next time I'll grab
the CVS version.)
Stas Bekman
I'm aware of only one issue with getppid, which is the main topic of this
thread. But Rafael has other thoughts in his reply and mod_perl is fixed
internally anyway, so it's up to you to decide whether it's worth
reverting/changing the geppid change.
> I'm not aware of a list, let alone a prioritised list.
__________________________________________________________________
Stas Bekman
> Stas Bekman wrote:
>
>>Nicholas Clark wrote:
>>
>>>On IRC Rafael said that he believes that his patch solves the problem
>>>without needing changes to perl. Is this the consensus view?
>>
>>Rafael's patch solves the problem in mod_perl 2.0. The problem still exists
>>for any other application that embeds perl and does the fork on its own (not
>>using pp_fork). So 5.8.2 (same as 5.8.1) may break applications that ran OK
>>under 5.8.0. I've suggested a patch to pp_getppid that fixes this problem in perl.
>
>
> On the other hand, the problem exists for $$ (there is no getpid function
> in perl) in all perls. So all applications that embed perl and that fork need
> at least to save and restore the PID (mod_perl 1 & 2 do this.) It's up to
> Nicholas to see if he wants to restore the 5.8.0 behavior in 5.8.1 but I think
> that PL_ppid will stay in 5.10.
Agreed
> I don't think your proposed patch works in the case where perl does a
> getppid(), so it marks it as cached, then the embedding application does
> a fork, and the PL_ppid_cached flag stays to "true".
Indeed, it won't work.
> (Oh, and sorry for my weak mod_perl patch-fu :) Next time I'll grab
> the CVS version.)
No problem at all ;)
Stas Bekman
Thanks, Rafael, committed with this correction.
BTW, there is the modperl cvs list if you want to see the cvs commits, or you
can observe them in the archives:
http://perl.apache.org/maillist/cvs.html