homepage

In current trunk (60097). Return from fork is not int but pid_t. 
Treating this as an int causes total breakage on systems with 64-bit pids.

Can you provide a patch? It would be good if it still worked on systems
that don't have pid_t (although we could also require pid_t and then see
what systems that breaks for).
Also, what systems use 64-bit pids?

Yeah; I shuold be able to provide one. I just hacked 2.4.4 to work so I
think I could provide a fix easily. The version I put together here is
rough, so I'll try to create a cleaner solution tonight or this weekend.
HiStar does (http://www.scs.stanford.edu/histar/).
Not sure if it's better to test for pid_t (in configure) or just use a
long long value all around a punt the problem? I'll probably just go
with the former unless there are objections.

It's probably better to use pid_t through the code and add a typedef int
pid_t for system's without pid_t. Or better typedef long pid_t?
On my Linux, pid_t is defined as __PID_T_TYPE, which is defines as
__S32_TYPE which ends up as int.

Don't use long long literally; not all systems have such a type, and
even if they do, they might not call it "long long".

I fixed the bug in r60484. Python was already using pid_t on several
occasions like getpid(). I added a test to verify that PyInt_FromLong()
can always handle pid_t.
Do you want to port the fix to 2.5?

> Do you want to port the fix to 2.5?
I'm not quite sure that the patch actually fixes the
problem. IIUC, HiStar is available in a 32-bit version,
too, yet it may still use a 64-bit pid_t (Ryan, can
you confirm whether that's the case?).
If so, Python would now fail to compile under that
patch. Backporting a change that causes Python to fail
to compile on some systems is not a good idea.
If that aspect was fixed also (e.g. by always returning
long ints on systems where sizeof(pid_t)>sizeof(long)),
a backport would be ok. For a perfect backport, that
change might still cause a behavior change: on
a system where sizeof(pid_t)>sizeof(long), yet the
system only ever uses pid_t values < INT_MAX, people
would see that the fork return type changes unreasonably;
a perfect backport would only return longs if the values
are out of range. This is probably over-cautious, as
it's fairly unlikely that such systems actually exist.

> IIUC, HiStar is available in a 32-bit version,
> too, yet it may still use a 64-bit pid_t (Ryan, can
> you confirm whether that's the case?).
Great point. pid_t is always 64-bit on HiStar.

Actually the current trunk of as of this morning (60484) is still broken
in a couple of ways. First, converting the pid_t using PyInt is a
problem and second the waitpids aren't corrected. This would cause
waits on invalid pids.

Ryan Stutsman wrote:
> Actually the current trunk of as of this morning (60484) is still broken
> in a couple of ways. First, converting the pid_t using PyInt is a
> problem and second the waitpids aren't corrected. This would cause
> waits on invalid pids.
I'll see what I kind do about it.
Christian

Martin v. Löwis wrote:
> If so, Python would now fail to compile under that
> patch. Backporting a change that causes Python to fail
> to compile on some systems is not a good idea.
I added the size comparison to identify systems with sizeof(pid_t) >
sizeof(long).
> If that aspect was fixed also (e.g. by always returning
> long ints on systems where sizeof(pid_t)>sizeof(long)),
> a backport would be ok. For a perfect backport, that
> change might still cause a behavior change: on
> a system where sizeof(pid_t)>sizeof(long), yet the
> system only ever uses pid_t values < INT_MAX, people
> would see that the fork return type changes unreasonably;
> a perfect backport would only return longs if the values
> are out of range. This is probably over-cautious, as
> it's fairly unlikely that such systems actually exist.
Your proposal looks sound and good to me, but it involves some work. The
chance would require a new format operator 'p' for argument parsing and
new functions like PyInt_FromPid_t() and PyInt_AsPid_t().
In r60504 I've changed the type for the remaining functions like waitpit
 and getsid to pid_t. It should make it easy to spot the lines that have
to be changed.
Christian

> Your proposal looks sound and good to me, but it involves some work. The
> chance would require a new format operator 'p' for argument parsing and
> new functions like PyInt_FromPid_t() and PyInt_AsPid_t().
No, it doesn't require that. You could use conditional compilation
throughout to achieve this, and the function might be more list
int_from_pid_t (i.e. static).

Is this pending for trunk and py3k too or just for 2.5 (and hence can be
closed)?

No, I don't think this is actually fixed in any version of Python at the
moment. The title may be a bit misleading, because all the versions now
store the result of fork in a pid_t and return it as a PyLong. However,
posix_waitpid is still pulling pid's as a PyInt. Changing this to
PyLong would probably work for our purposes, but I guess the hangup was
that in reality pid_t is supposed to be an opaque datatype and
implementing it in CPython is non-trivial and has little benefit.
Perhaps we close this as won't fix or I can create a patch to at least
give a hack for 64-bit pid's but still treated as a long.

The following patch should do the trick.

The patch looks fine to me, please apply.

Thanks, the patch was committed in r72852, r72853, r72854.
I then realized a couple of functions had been looked over (including
getpid() and getppid()), and committed a complement in r72855, r72856
and r72858.