[roland/getpid] Simplify getpid handling of the race case.

  In looking at consolidating the NPTL vs not versions of vfork, I came
across the single, bizarre difference between the libpthread and libc
versions of vfork.  This is implemented by a few lines of assembly for each
machine, and nowhere is there a comment that explains the rationale.  

The code in question is fiddling with the pid field of 'struct pthread'.
It negates the value before doing vfork and then restores it afterwards in
the parent.  The rationale for this is that there could be a race where a
signal handler runs in the child immediately after the vfork call, and in
that case the getpid optimization of just returning THREAD_SELF->pid would
return the parent's PID rather than the child's.  That's all fair enough.
Nothing ever sets ->pid in the child; it wouldn't be safe to do so, since
the child and parent share memory, so if ->pid were ever >0 in the child,
there would be a race after the child exits/execs and the parent resumes
where a signal handler could run and see the child-set value of ->pid.
(This just means that getpid's short-circuit logic will never win in a
vfork child, so it makes actual getpid system calls.)

The difference is that in the libc version, it checks for ->pid being zero
and if it's zero instead sets it to 0x80000000 (i.e. an arbitrary negative
number).  The libpthread version doesn't do that, so if it's set to zero
then it stays that way.  AFAICT the reason for this is so that getpid, in
its handling of the race case (i.e. THREAD_SELF->pid <= 0) can have another
special case to avoid making the getpid system call: If ->pid was exactly
zero but ->tid is nonzero, it just returns ->tid as the PID.  But I don't
actually see how ->pid could ever be set to zero so as to trigger this
case.  Then there is further logic whereby if ->pid and ->tid were both set
to zero, then getpid will store the PID retrieved by the system call in
->tid (but not in ->pid).  Again I don't see how that case can arise: ->tid
can never be zero in a live thread after __pthread_initialize_minimal runs.
Perhaps there can be a window where user constructors run before that?

It all seems rather muddled, and I feel like I must be missing some of
what's going on.  If anything, the libc/libpthread difference seems
backwards: in libc without libpthread, using ->tid should be OK because
there's only one thread and its TID==PID; in libpthread, using ->tid can
only be right if you're sure you're in the child and ->tid has been set to
the child's new TID (which matches its PID).  Maybe someone else can help
explain it all.

But all this complexity seems like a lot of hair to micro-optimize a case
that can only arise during a race condition.  If we simplify getpid so that
it always just falls back to the system call when any fork/vfork is in
progress, then we don't need the complexity and we don't need the libc and
libpthread versions of vfork to differ.  This change does that, leaving the
way open to consolidate the vfork code during my NPTL consolidation.

I'm not very clear on the utility of ever setting ->pid to a negative
value.  The fork and vfork code never look at the negated value again, they
just save the original value locally and restore it later.  So why not just
zero ->pid at the beginning of fork and vfork instead?

Does this seem sensible?  Can anyone give a full explanation of the picture
here that gives the rationale for the confusing things in the status quo?

Thanks,
Roland

	* nptl/sysdeps/unix/sysv/linux/getpid.c
	(really_getpid): Function removed.
	(__getpid): Rewritten.  Under [!NOT_IN_libc], use THREAD_SELF->pid if
	it's > 0.  Otherwise always just make the system call.

[roland/getpid] Simplify getpid handling of the race case.

Commit Message

Comments

Patch