[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Ross Lagerwall]

Ross Lagerwall  added the comment:

The tests seem to be working fine on all the buildbots now...

Thanks!

--
resolution:  -> accepted
stage: commit review -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[STINNER Victor]

STINNER Victor  added the comment:

> But I think we could just remove this test

The test pass on Linux and FreeBSD 6 using a subprocess. I commited my patch to 
replace fork() by subprocess, let's see how it works on buildbots.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Roundup Robot]

Roundup Robot  added the comment:

New changeset 00ca0c2c7bc0 by Victor Stinner in branch 'default':
Issue #12303: run sig*wait*() tests in a subprocesss
http://hg.python.org/cpython/rev/00ca0c2c7bc0

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Charles-François Natali]

Charles-François Natali  added the comment:

> Oh, the problem is that sigwait() behaviour changes after a fork: it is 
> interrupted if an unexpected signal is received, but the signal handler is 
> not called. It behaves correctly (the signal handler is called) without the 
> fork.
>

Reminds me of http://bugs.python.org/issue8407#msg138066

But I think we could just remove this test: honestly, I'm not sure
that checking the behavior in case of delivery of an unblocked signal
is really useful, especially since the behavior is not clearly
defined, see POSIX man page:
"""
The sigwaitinfo() and sigtimedwait() functions may fail if:

[EINTR]
The wait was interrupted by an unblocked, caught signal. It will be
documented in system documentation whether this error will cause these
functions to fail.
"""

"may fail" [...] "It will be documented in system documentation"

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[STINNER Victor]

STINNER Victor  added the comment:

> test_sigwaitinfo_interrupted() fails because SIGALRM
> signal handler is called ...

Oh, the problem is that sigwait() behaviour changes after a fork: it is 
interrupted if an unexpected signal is received, but the signal handler is not 
called. It behaves correctly (the signal handler is called) without the fork.

The correct fix is maybe to avoid the os.fork() instead of skipping the test on 
FreeBSD 6.

--
nosy: +neologix
stage: patch review -> commit review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[STINNER Victor]

STINNER Victor  added the comment:

> @Victor, you've had some experience with fixing signals
> on the FreeBSD 6 buildbot...

It's not exactly that I had some experience, it's just that I have a SSH access 
to the buildbot.

The following code hangs for (exactly?) 30 seconds on sigwaitinfo():

import os, signal, threading
s = signal.SIGALRM
signal.pthread_sigmask(signal.SIG_BLOCK, [s])
os.kill(os.getpid(), s)
signal.sigwaitinfo([s])
signal.pthread_sigmask(signal.SIG_UNBLOCK, [s])

sigwait() and sigtimedwait() wait also 30 seconds.

The following code only hangs for 1 second using sigwait(), sigwaitinfo() or 
sigtimedwait():

import os, signal, threading
s = signal.SIGALRM
signal.pthread_sigmask(signal.SIG_BLOCK, [s])
os.kill(os.getpid(), s)
signal.sigwaitinfo([s])
signal.pthread_sigmask(signal.SIG_UNBLOCK, [s])


test_sigtimedwait_poll() should be skipped on FreeBSD 6, there is a bug in the 
OS.

test_sigwaitinfo_interrupted() fails because SIGALRM signal handler is called, 
and the default FreeBSD handler stops the process. You should install a dummy 
signal handler (e.g. lambda signum, frame: None) for SIGALRM. I don't 
understand why the test doesn't fail on Linux, the default handler of SIGALRM 
on Linux stops also the process.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Ross Lagerwall]

Ross Lagerwall  added the comment:

>From the FreeBSD 6.4 buildbot:
"""
[172/356/2] test_signal
error: 'NoneType' object has no attribute 'si_signo'
error: False is not true
test test_signal failed -- multiple errors occurred; run in verbose mode for 
details
Re-running test test_signal in verbose mode
test_getsignal (test.test_signal.PosixTests) ... ok
test_out_of_range_signal_number_raises_error (test.test_signal.PosixTests) ... 
ok
test_setting_signal_handler_to_none_raises_error (test.test_signal.PosixTests) 
... ok
test_main (test.test_signal.InterProcessSignalTests) ... skipped 'inter process 
signals not reliable (do not mix well with threading) on freebsd6'
test_pending (test.test_signal.WakeupSignalTests) ... ok
test_pthread_kill_main_thread (test.test_signal.WakeupSignalTests) ... ok
test_signum (test.test_signal.WakeupSignalTests) ... ok
test_wakeup_fd_during (test.test_signal.WakeupSignalTests) ... ok
test_wakeup_fd_early (test.test_signal.WakeupSignalTests) ... ok
test_siginterrupt_off (test.test_signal.SiginterruptTest) ... ok
test_siginterrupt_on (test.test_signal.SiginterruptTest) ... ok
test_without_siginterrupt (test.test_signal.SiginterruptTest) ... ok
test_itimer_exc (test.test_signal.ItimerTest) ... ok
test_itimer_prof (test.test_signal.ItimerTest) ... skipped 'itimer not reliable 
(does not mix well with threading) on freebsd6'
test_itimer_real (test.test_signal.ItimerTest) ... ok
test_itimer_virtual (test.test_signal.ItimerTest) ... skipped 'itimer not 
reliable (does not mix well with threading) on some BSDs.'
test_issue9324 (test.test_signal.WindowsSignalTests) ... skipped 'Windows 
specific'
test_pthread_kill (test.test_signal.PendingSignalsTests) ... ok
test_pthread_sigmask (test.test_signal.PendingSignalsTests) ... ok
test_pthread_sigmask_arguments (test.test_signal.PendingSignalsTests) ... ok
test_sigpending (test.test_signal.PendingSignalsTests) ... ok
test_sigpending_empty (test.test_signal.PendingSignalsTests) ... ok
test_sigtimedwait (test.test_signal.PendingSignalsTests) ... ok
test_sigtimedwait_negative_timeout (test.test_signal.PendingSignalsTests) ... ok
test_sigtimedwait_poll (test.test_signal.PendingSignalsTests) ... error: 
'NoneType' object has no attribute 'si_signo'
FAIL
test_sigtimedwait_timeout (test.test_signal.PendingSignalsTests) ... ok
test_sigwait (test.test_signal.PendingSignalsTests) ... ok
test_sigwait_thread (test.test_signal.PendingSignalsTests) ... ok
test_sigwaitinfo (test.test_signal.PendingSignalsTests) ... ok
test_sigwaitinfo_interrupted (test.test_signal.PendingSignalsTests) ... error: 
False is not true
FAIL

==
FAIL: test_sigtimedwait_poll (test.test_signal.PendingSignalsTests)
--
Traceback (most recent call last):
  File 
"/usr/home/db3l/buildarea/3.x.bolen-freebsd/build/Lib/test/test_signal.py", 
line 685, in test_sigtimedwait_poll
self.wait_helper(test, self.handler, signal.SIGALRM)
  File 
"/usr/home/db3l/buildarea/3.x.bolen-freebsd/build/Lib/test/test_signal.py", 
line 645, in wait_helper
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
AssertionError: Tuples differ: (55455, 256) != (55455, 0)

First differing element 1:
256
0

- (55455, 256)
? ^^^

+ (55455, 0)
? ^


==
FAIL: test_sigwaitinfo_interrupted (test.test_signal.PendingSignalsTests)
--
Traceback (most recent call last):
  File 
"/usr/home/db3l/buildarea/3.x.bolen-freebsd/build/Lib/test/test_signal.py", 
line 721, in test_sigwaitinfo_interrupted
self.wait_helper(test, self.alarm_handler, signal.SIGUSR1)
  File 
"/usr/home/db3l/buildarea/3.x.bolen-freebsd/build/Lib/test/test_signal.py", 
line 645, in wait_helper
self.assertEqual(os.waitpid(pid, 0), (pid, 0))
AssertionError: Tuples differ: (55460, 256) != (55460, 0)

First differing element 1:
256
0

- (55460, 256)
? ^^^

+ (55460, 0)
? ^
"""




@Victor, you've had some experience with fixing signals on the FreeBSD 6 
buildbot.
Have you got any ideas as to what the cause of this may be?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Roundup Robot]

Roundup Robot  added the comment:

New changeset 768234f5c246 by Ross Lagerwall in branch 'default':
Fix test_signal on Windows after #12303.
http://hg.python.org/cpython/rev/768234f5c246

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Roundup Robot]

Roundup Robot  added the comment:

New changeset 1137f7021b95 by Ross Lagerwall in branch 'default':
Issue #12303: Add sigwaitinfo() and sigtimedwait() to the signal module.
http://hg.python.org/cpython/rev/1137f7021b95

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[STINNER Victor]

STINNER Victor  added the comment:

> If this is OK, I'll commit the patch.

Yep, this version is the good one. Please keep the issue open until the test 
pass on all buildbots. Each newly added signal functions took me some days to 
fix the test for Mac OS X and/or FreeBSD 6. But you are maybe more lucky than 
me ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Ross Lagerwall]

Ross Lagerwall  added the comment:

> What do you mean? signal.SIGPOLL exists in Python 3.3.

Right, si_band added.

> - test_sigwaitinfo(), test_sigtimedwait_poll(), 
> test_sigwaitinfo_interrupted() are called from a child process. In 
> test_wait(), I chose to write manually to stdout and call os._exit(1) (oh, I 
> forgot an explicit sys.stdout.flush()). I don't know if you can use TestCase 
> methods in a child process (I don't know what is written to stdout, 
> _wait_helper() calls os._exit).

I assume you mean stderr? Well anyway if the assert* fails, an exception is 
raised and then caught by this code in wait_helper():
"""
except BaseException as err:
print("error: {}".format(err), file=sys.stderr)
sys.stderr.flush()
os._exit(1)
"""
which prints it to stderr.
If this is OK, I'll commit the patch.

--
Added file: http://bugs.python.org/file22452/issue12303_v3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[STINNER Victor]

STINNER Victor  added the comment:

>> It is possible to pass a negative timeout
> It now raises an exception like select.

Great.

>> According to the manual page, sigwaitinfo() or sigtimedwait() 
>> can be interrupted (EINTR)
> Actually, PyErr_SetFromErrno() does this implicitly. I added a test case 
> anyway.

I would prefer an explicit call to PyErr_CheckSignals(), but if there is a 
test, it's just fine.

> So I've left out si_addr (I don't think it's needed)
> ... and si_value (not sure what it's for or how to represent a union :-))

I don't think that it means something in Python to have an address or this 
low-level "value" field. If someone needs them, we would need an use case with 
an example (to test it!). (So ok to not expose them)

> ... si_band (we don't have SIGPOLL)

What do you mean? signal.SIGPOLL exists in Python 3.3.

>> sigtimedwait() raises a OSError(EGAIN) on timeout.
> It now returns None.

Great.

--

>> I will do a deeper review on the second version of your patch :-)
> How much more in depth can it get ;-) ?

Here you have, issue12303_v2.patch:

 - "PyStructSequence_SET_ITEM(result, 4, PyLong_FromPid(si->si_uid));" looks 
wrong (we don't need to call PyLong_FromLongLong): si_uid type is uid_t, not 
pid_t. posix_getuid() simply uses PyLong_FromLong((long)getuid()).

 - sigwaitinfo() doc doesn't mention that the function can be interrupted if an 
unexpected signal is received. I don't know if it should be mentionned. It is 
mentionned in the manpage, in the ERRORS section (EINTR).

 - tests: test_sigtimedwait_negative_timeout() doesn't need to use 
_wait_helper() (which creates a subprocess!). You may also test (-1, 0) and (0, 
-1) timeouts.

 - test_sigwaitinfo(), test_sigtimedwait_poll(), test_sigwaitinfo_interrupted() 
are called from a child process. In test_wait(), I chose to write manually to 
stdout and call os._exit(1) (oh, I forgot an explicit sys.stdout.flush()). I 
don't know if you can use TestCase methods in a child process (I don't know 
what is written to stdout, _wait_helper() calls os._exit).

 - you may want to prepare the "What's new in Python 3.3" document (mention the 
2 new functions)

 - style: _wait_helper(): you don't have to mark the helper as private (-> 
wait_helper())

 - style: _fill_siginfo() is already a static function you don't need a "_" 
prefix

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Ross Lagerwall]

Ross Lagerwall  added the comment:

Thanks for the review.

> Why do you wait until the end of PyInit_signal() to set initialized to 1?
Fixed.

> If this variable is only present ... but I see that the init function of the 
> posixmodule.c has also a static initialized variable.
I simply followed the same format of the posix module.

> "(If one of the signals in set is already pending for the calling thread,  
> sigwaitinfo() will return immediately with information about that signal.)"
Added.

> "If both fields of this structure are specified as  0,  a  poll  is  
> performed:  sigtimedwait() returns  immediately,  either with information 
> about a signal that was pending for the caller, or with an error if none of 
> the signals in set was pending."
Added.

> The manpage doesn't tell that the signal handler is not called, should we say 
> it in the Python doc?
Added - let's rather be more explicit.

> Doc: you may add links between pause(), sigwait(), sigwaitinfo() and 
> sigtimedwait() functions.
Added.

> The timeout is a tuple. Usually, Python uses float for timeout (e.g. see 
> select.select). I suppose that you chose a tuple to keep the precision of the 
> timespec structure. We may accept both types: (sec: int, nanosec: int) or 
> sec: float. It would be nice to have the opinion of our time expect, 
> Alexander Belopolsky.
The are some uses of this tuple that were added in #10812 (e.g. futimens) that 
was reviewed by Alexander.
However, there was a conflict about it at #11457.
I'd say for now, let's keep it as a tuple and if a decision is eventually made 
as to how to
represent nanosecond timestamps, we can change them all then.

> It is possible to pass a negative timeout
It now raises an exception like select.

> signal_sigwaitinfo() and signal_sigtimedwait() use iterable_to_sigset()
Fixed.

> According to the manual page, sigwaitinfo() or sigtimedwait() can be 
> interrupted (EINTR)
Actually, PyErr_SetFromErrno() does this implicitly. I added a test case anyway.

> Your patch doesn't touch configure nor pyconfig.h.in, only configure.in.
I prefer to keep the generated changes out of the patch. When I commit, I'll 
run autoreconf.

> siginfo_t structure contains more fields, but I don't know if we need all of 
> them. It can be improved later.
POSIX 2008 specifies:
int   si_signo  Signal number. 
int   si_code   Signal code. 
int   si_errno  If non-zero, an errno value associated with 
this signal, as described in . 
pid_t si_pidSending process ID. 
uid_t si_uidReal user ID of sending process. 
void *si_addr   Address of faulting instruction. 
int   si_status Exit value or signal. 
long  si_band   Band event for SIGPOLL. 
union sigval  si_value  Signal value.

So I've left out si_addr (I don't think it's needed), si_band (we don't have 
SIGPOLL)
and si_value (not sure what it's for or how to represent a union :-)).

> sigtimedwait() raises a OSError(EGAIN) on timeout.
It now returns None.

> test_sigtimedwait_timeout(): why do you call signal.alarm()?
Copy/paste error.

> You may also add a test for timeout=0, I suppose that it is a special timeout 
> value.
Added.

> I will do a deeper review on the second version of your patch :-)
How much more in depth can it get ;-) ?

--
Added file: http://bugs.python.org/file22439/issue12303_v2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[STINNER Victor]

STINNER Victor  added the comment:

Cool, a patch! "Some" comments.

Why do you wait until the end of PyInit_signal() to set initialized to 1? If 
this variable is only present to call PyStructSequence_InitType() only once, 
you can set initialized to 1 directly in the if. Is it possible that 
PyInit_signal() is called more than once? C modules cannot be unloaded in 
Python, but I see that the init function of the posixmodule.c has also a static 
initialized variable.

Doc: The sigwaitinfo()/sigtimedwait() manual page contains interesting infos:

"(If one of the signals in set is already pending for the calling thread,  
sigwaitinfo() will return immediately with information about that signal.)"

"If both fields of this structure are specified as  0,  a  poll  is  performed: 
 sigtimedwait() returns  immediately,  either with information about a signal 
that was pending for the caller, or with an error if none of the signals in set 
was pending."

The manpage doesn't tell that the signal handler is not called, should we say 
it in the Python doc?

Doc: you may add links between pause(), sigwait(), sigwaitinfo() and 
sigtimedwait() functions. We should maybe reorganise the signal doc to group 
functions. We need maybe a section for "pending signals" functions, functions 
to block or wait signals: pause(), pthread_sigmask(), sigpending(), sigwait(), 
sigwaitinfo(), sigtimedwait(). Another big theme of the signal module is signal 
handling. We may group functions by these two themes. Well, it is better to 
reorganize the doc is a second commit ;-)

The timeout is a tuple. Usually, Python uses float for timeout (e.g. see 
select.select). I suppose that you chose a tuple to keep the precision of the 
timespec structure. We may accept both types: (sec: int, nanosec: int) or sec: 
float. It would be nice to have the opinion of our time expect, Alexander 
Belopolsky.

It is possible to pass a negative timeout: the select() functions accepts or 
not negative timeout depending on the platform. In Python 3.3, select.select() 
now always raise an exception if the timeout is negative to have the same 
behaviour on all platforms. According to the Linux manual page, sigtimedwait() 
can return EINVAL if the timeout is invalid. We may also always raise an 
exception if the timeout is negative in sigtimedwait()?

signal_sigwaitinfo() and signal_sigtimedwait() use iterable_to_sigset() whereas 
this function is only compiled if "#if defined(PYPTHREAD_SIGMASK) || 
defined(HAVE_SIGWAIT)". You have to fix this test.

According to the manual page, sigwaitinfo() or sigtimedwait() can be 
interrupted (EINTR) by an unexpected signal (in signal not the signal set): you 
should call PyErr_CheckSignals(). You should add a test for this case.

Your patch doesn't touch configure nor pyconfig.h.in, only configure.in. Edit 
configure manually (better to limit the size of the patch) and run autoheader 
to regenerate pyconfig.h.in (or maybe edit manually pyconfig.h.in).

siginfo_t structure contains more fields, but I don't know if we need all of 
them. It can be improved later.

sigtimedwait() raises a OSError(EGAIN) on timeout. The behaviour must be 
documented, or we can choose another behaviour. We may simply return None in 
case of a timeout, it is just more practical to use than a try/except. For 
example, threading.Lock.acquire(timeout) simply returns False in case of a 
timeout. select.select() returns ([], [], []) in case of a timeout, not an 
exception.

test_sigtimedwait_timeout(): why do you call signal.alarm()? You may also add a 
test for timeout=0, I suppose that it is a special timeout value.

I will do a deeper review on the second version of your patch :-) Thanks again 
for the patch. I tried to write it, but I was bored of the siginfo_t fields 
(too many fields, and I didn't know how to expose them in Python).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Ross Lagerwall]

Ross Lagerwall  added the comment:

Here's a patch to add the two functions (with docs and tests).

You'll need to run autoreconf before compiling.

--
assignee:  -> rosslagerwall
keywords: +patch
nosy: +rosslagerwall
stage:  -> patch review
type:  -> feature request
Added file: http://bugs.python.org/file22434/issue12303.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[Giampaolo Rodola']

Changes by Giampaolo Rodola' :


--
nosy: +giampaolo.rodola

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue12303] expose sigwaitinfo() and sigtimedwait() in the signal module

[STINNER Victor]

New submission from STINNER Victor :

Python 3.3 adds timeout arguments to communicate() and wait() methods of 
subprocess.Popen, acquire() method of threading.Lock, and maybe more. I like 
(optional) timeouts, and so it would be nice to have sigtimedwait().

If we expose sigtimedwait(), it's trivial to expose sigwaitinfo(), so should 
expose both.

--
components: Extension Modules
messages: 138033
nosy: haypo
priority: normal
severity: normal
status: open
title: expose sigwaitinfo() and sigtimedwait() in the signal module
versions: Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com