msg61926 - (view) |
Author: Ryan Stutsman (stutsman) |
Date: 2008-01-31 20:07 |
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.
|
msg61927 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2008-01-31 20:26 |
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?
|
msg61928 - (view) |
Author: Ryan Stutsman (stutsman) |
Date: 2008-01-31 20:51 |
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.
|
msg61931 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2008-01-31 21:30 |
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.
|
msg61932 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2008-01-31 21:32 |
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".
|
msg61939 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2008-01-31 23:09 |
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?
|
msg61947 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2008-02-01 05:04 |
> 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.
|
msg61948 - (view) |
Author: Ryan Stutsman (stutsman) |
Date: 2008-02-01 06:06 |
> 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.
|
msg61965 - (view) |
Author: Ryan Stutsman (stutsman) |
Date: 2008-02-01 16:32 |
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.
|
msg61966 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2008-02-01 16:54 |
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
|
msg61972 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2008-02-01 19:18 |
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
|
msg61974 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2008-02-01 19:39 |
> 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).
|
msg87717 - (view) |
Author: Daniel Diniz (ajaksu2) *  |
Date: 2009-05-13 22:02 |
Is this pending for trunk and py3k too or just for 2.5 (and hence can be
closed)?
|
msg88058 - (view) |
Author: Ryan Stutsman (rstutsman) |
Date: 2009-05-18 23:17 |
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.
|
msg88073 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2009-05-19 07:36 |
The following patch should do the trick.
|
msg88228 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2009-05-23 11:11 |
The patch looks fine to me, please apply.
|
msg88241 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2009-05-23 16:22 |
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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:30 | admin | set | github: 46275 |
2009-05-23 16:22:03 | pitrou | set | status: open -> closed resolution: accepted -> fixed messages:
+ msg88241
|
2009-05-23 11:11:47 | loewis | set | resolution: accepted messages:
+ msg88228 |
2009-05-19 07:37:10 | pitrou | set | stage: patch review resolution: fixed -> (no value) versions:
+ Python 2.6, Python 3.0, Python 3.1, Python 2.7, - Python 2.5 |
2009-05-19 07:36:48 | pitrou | set | files:
+ issue1983.patch
nosy:
+ pitrou messages:
+ msg88073
keywords:
+ patch |
2009-05-18 23:17:46 | rstutsman | set | nosy:
+ rstutsman messages:
+ msg88058
|
2009-05-13 22:02:56 | ajaksu2 | set | status: pending -> open nosy:
+ ajaksu2 messages:
+ msg87717
|
2009-05-13 22:02:51 | ajaksu2 | link | issue2005 dependencies |
2008-02-01 19:39:56 | loewis | set | messages:
+ msg61974 |
2008-02-01 19:18:27 | christian.heimes | set | messages:
+ msg61972 |
2008-02-01 16:54:47 | christian.heimes | set | messages:
+ msg61966 |
2008-02-01 16:32:16 | stutsman | set | messages:
+ msg61965 |
2008-02-01 06:07:00 | stutsman | set | messages:
+ msg61948 |
2008-02-01 05:04:53 | loewis | set | messages:
+ msg61947 |
2008-01-31 23:09:38 | christian.heimes | set | status: open -> pending resolution: fixed messages:
+ msg61939 versions:
+ Python 2.5, - Python 2.6, Python 3.0 |
2008-01-31 21:32:58 | loewis | set | messages:
+ msg61932 |
2008-01-31 21:30:02 | christian.heimes | set | priority: normal nosy:
+ christian.heimes type: behavior messages:
+ msg61931 |
2008-01-31 20:51:28 | stutsman | set | messages:
+ msg61928 |
2008-01-31 20:26:58 | loewis | set | nosy:
+ loewis messages:
+ msg61927 |
2008-01-31 20:07:27 | stutsman | create | |