classification
Title: setitimer, getitimer wrapper
Type: enhancement Stage:
Components: Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: loewis Nosy List: georg.brandl, gpolo, loewis, schmir, tleeuwenburg@gmail.com, trent
Priority: normal Keywords: patch

Created on 2008-03-05 14:42 by gpolo, last changed 2009-04-08 10:50 by tleeuwenburg@gmail.com. This issue is now closed.

Files
File name Uploaded Description Edit
setitimer_getitimer.diff gpolo, 2008-03-08 10:56 complete patch
trunk-itimer.txt schmir, 2008-03-10 09:00 patch backported to trunk
Messages (22)
msg63277 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-05 14:42
Right now Python misses a wrapper for setitimer and getitimer and I
believe it would be interesting to include them. I'm (almost) sure some
other people may find it useful too.

I'm attaching a standalone module, but if it gets to be included in
Python, I think it would be better to create a patch against signal
module. Also, its tests are pretty poor. 

Improvements are welcomed.
msg63278 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-05 14:46
I forgot to remove an unwanted comment from it =)
Attaching new version.
msg63293 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-03-05 20:36
I can see nothing wrong with including setitimer support for systems
where it is available, and I agree that the signal module would be the
right place.

So whoever wants to complete it, feel free to produce a complete patch
against the trunk.
msg63295 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-05 21:43
Martin, thanks for supporting the idea.

I'm attaching a patch. It is against rev 61255, py3k branch.
It patches configure, configure.in, Modules/signalmodule.c and pyconfig.h.in
I wasn't sure if I should attach a diff for each file, so they are all
packed in the same diff. Also, I wasn't sure if I should append the 
"configure" diff, but I did.

Documentation in "Doc/" and tests aren't done yet, if someone want to
pick this task, please say it.
msg63297 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-05 21:55
I noticed that I forgot to change setitimer and getitimer functions from
itimer_setitimer to signal_setitimer (same for getitimer).

I'm attaching a patch that should be applied after the previous one to
do this renaming.
msg63306 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-03-06 06:59
Patch review:
* ItimerError should be signal.ItimerError, not signal.error.
* It should probably inherit from EnvironmentError or IOError.
* itimer_retval will leak the new tuple if one of the PyFloat_FromDouble
fails.
* Do you have test and doc patches, too?
* What does the "which" argument mean, is it an arbitrary integer
identifying the timer?
msg63315 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-06 10:12
Thanks for the review, corrections and suggestions Georg.

For the first three items: I will be working on this later today.

For the last item: "which" is one of those three new constants added. I
believe if you pass something else setitimer/getitimer might raise an
error (not tested).

I still have to write doc and tests.
msg63319 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-06 11:48
I'm attaching another patch, this should be applied after the other ones
have been applied. It fixes what Georg mentioned. 
I have chosen to let ItimerError inherit from IOError, and improved the
docstring of setitimer.
msg63320 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-06 13:26
Updated Doc/library/signal.rst, follows a patch.
msg63325 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-06 16:26
I've done some tests for getitimer/setitimer, diff is attached.
msg63390 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2008-03-08 09:56
Could you please put all changes in one complete patch? It's much easier
to review that way.
msg63397 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-08 10:56
Complete patch attached
msg63398 - (view) Author: Ralf Schmitt (schmir) Date: 2008-03-08 13:08
I'd like to also see this in 2.6. Should I update the patch (if it
doesn't apply) and test? (I guess the signal module hasn't changed that
much).
msg63399 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-03-08 13:10
If you are going to backport it to 2.6, then the C wrapper should be
adapted to match Python 2.x C coding style. If the other parts don't
apply correctly, then you should update it aswell.
msg63437 - (view) Author: Ralf Schmitt (schmir) Date: 2008-03-10 09:00
Okay, the patch applies just fine (besides configure, which must be
regenerated). Running the tests however consumes 3 gb of memory.
The range functions must be changed to xrange on trunk. I've also
changed the docs a bit ("new in 2.6").
I didn't change anything related to coding standards (is this an issue
with tabs vs spaces?).
all tests in test_signal work with a patched revision 61332 of trunk, on
a 64 bit linux.
msg64413 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-03-24 13:40
Thanks for the patch. Committed as r61847 and r61848
msg64999 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-04-05 18:55
Reopening. Apparently, the tests fail on FreeBSD; see

http://www.python.org/dev/buildbot/trunk/x86%20FreeBSD%203%20trunk/builds/77/step-test/0

Can you please look into this?
msg65002 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-04-05 19:31
I'm investigating the problem loewis, thanks for reporting. But it would
be better if someone with running FreeBSD could help me there, in case I
find the cause for this.

Also some changes were made to the original patch:

neal.norwitz did a commit where he says:
"Using a negative time causes Linux to treat it as zero, so disable that
test."
That is not what I get here, maybe a very different kernel, anyway, I
believe he could have mentioned this here.

jeffrey.yasskin said:
".. fix some flakiness in test_itimer_prof, which could detect that the
timer had reached 0 before the signal arrived announcing that fact."

followed by these changes:

signal.setitimer(self.itimer, 0.2)        (old)
signal.setitimer(self.itimer, 0.2, 0.2)   (new) -> not sure the reason
for this change

and added:
self.assertEquals(signal.getitimer(self.itimer), (0.0, 0.0)) -> this is
the same test I did for itimer_virtual, and it is a bit questionable it
is really useful at all. I don't understand how these changes matches
what he comments on his commit.
msg65004 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-04-05 19:55
FreeBSD's man page for setitimer/getitimer doesn't look different from
the one in Linux. 
But.. both tests assumes the computer is not so fast that it finishes
"for i in xrange(100000000)" before the timer expire, maybe it was not
the case in that machine. Both timers (virtual and prof) are set to
expire after 0.2 seconds, and for itimer_virtual it restarts the timer 3
times before setting it to 0.
If someone has a freebsd machine (a fast one apparently) that could run
some tests and help me on this, it would be very nice.

Also sorry for various typos in my last message.
msg65129 - (view) Author: Guilherme Polo (gpolo) * (Python committer) Date: 2008-04-07 23:28
Trent Nelson kindly gave me access to his FreeBSD 6.2 buildbot so I had
chance to do some tests. The problem happens when Python is built
against or libc_r, or if you are using libmap you won't need to
recompile but the problem still happens when using libc_r.

I started searching in the FreeBSD bug tracker and found this issue:
http://www.freebsd.org/cgi/query-pr.cgi?pr=threads/49087 which seems
very similar to the problem related here.

I've also done a very simple "test" in C, just to demonstrate that this
issue isn't related to Python at all:

#include <stdio.h>
#include <signal.h>
#include <sys/time.h>

void h(int signo)
{
    struct itimerval t;

    getitimer(ITIMER_PROF, &t);
    printf("%d %d\n", t.it_value.tv_sec, t.it_value.tv_usec);

    printf("deactive ITIMER_PROF\n");
    t.it_value.tv_sec = 0;
    t.it_value.tv_usec = 0;
    setitimer(ITIMER_PROF, &t, &t);
}

int main(void)
{
    struct itimerval ival;

    ival.it_value.tv_sec = 1;
    ival.it_value.tv_usec = 0;
    ival.it_interval.tv_sec = 1;
    ival.it_interval.tv_usec = 0;

    signal(SIGPROF, h);
    printf("%d\n", setitimer(ITIMER_PROF, &ival, NULL));
    alarm(2);

    while (1) {
        getitimer(ITIMER_PROF, &ival);
        if (ival.it_value.tv_sec == 0 && ival.it_value.tv_usec == 0)
            break;
        }

    return 0;
}

When I compile this using -lc_r then the callback "h" is never called
and then the alarm is fired. Compiling against pthread, thr or nothing
(since this example doesn't need any threading library) doesn't
demonstrate this problem and all is fine (callback "h" is invoked,
infinite loop finishes and test returns 0).

Should further discussion be moved to python-dev ? I'm somewhat stuck on
how to resolve this, besides saying to upgrade to FreeBSD 7 which uses
libthr by default.
msg65144 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-04-08 05:19
> Should further discussion be moved to python-dev ? I'm somewhat stuck on
> how to resolve this, besides saying to upgrade to FreeBSD 7 which uses
> libthr by default.

Discussing on python-dev is fine. An acceptable solution would be to
omit setitimer/getitimer on FreeBSD if the FreeBSD version is "wrong",
either at configure time or at compile time. An even better solution
would be to test at configure time whether these functions work, and
refuse to expose them if they don't (not sure whether that's possible,
though, as I don't know when the choice of threading library is made).
msg85768 - (view) Author: Tennessee Leeuwenburg (tleeuwenburg@gmail.com) Date: 2009-04-08 10:50
Closing this issue as the main functionality is committed. See issue 
5722 for follow-up regarding FreeBSD functionality.
History
Date User Action Args
2009-04-08 10:50:46tleeuwenburg@gmail.comsetstatus: open -> closed
nosy: + tleeuwenburg@gmail.com
messages: + msg85768

2008-04-12 16:24:00gpolosetfiles: - setitimer_getitimer_wrapper_test.diff
2008-04-12 16:23:55gpolosetfiles: - setitimer_getitimer_wrapper_doc_update.diff
2008-04-12 16:23:49gpolosetfiles: - setitimer_getitimer_wrapper_doc.diff
2008-04-12 16:23:18gpolosetfiles: - setitimer_getitimer_wrapper_fixes.diff
2008-04-12 16:22:51gpolosetfiles: - setitimer_getitimer_wrapper_rename.diff
2008-04-12 16:22:29gpolosetfiles: - setitimer_getitimer_wrapper.diff
2008-04-12 16:22:17gpolosetfiles: - py-itimer-0.1.2.tar.gz
2008-04-10 22:35:44trentsetnosy: + trent
2008-04-08 05:19:16loewissetmessages: + msg65144
2008-04-07 23:28:37gpolosetmessages: + msg65129
2008-04-05 19:55:04gpolosetmessages: + msg65004
2008-04-05 19:31:37gpolosetmessages: + msg65002
2008-04-05 18:55:43loewissetstatus: closed -> open
messages: + msg64999
2008-03-24 13:40:46loewissetstatus: open -> closed
resolution: accepted
messages: + msg64413
2008-03-10 09:02:08schmirsetversions: + Python 2.6
2008-03-10 09:00:54schmirsetfiles: + trunk-itimer.txt
messages: + msg63437
2008-03-08 13:10:49gpolosetmessages: + msg63399
2008-03-08 13:08:16schmirsetmessages: + msg63398
2008-03-08 10:56:10gpolosetfiles: + setitimer_getitimer.diff
messages: + msg63397
2008-03-08 09:56:42georg.brandlsetassignee: loewis
messages: + msg63390
2008-03-07 17:11:44schmirsetnosy: + schmir
2008-03-06 16:26:16gpolosetfiles: + setitimer_getitimer_wrapper_test.diff
messages: + msg63325
2008-03-06 16:23:27gpolosetfiles: + setitimer_getitimer_wrapper_doc_update.diff
2008-03-06 13:26:38gpolosetfiles: + setitimer_getitimer_wrapper_doc.diff
messages: + msg63320
2008-03-06 11:48:15gpolosetfiles: + setitimer_getitimer_wrapper_fixes.diff
messages: + msg63319
2008-03-06 10:12:25gpolosetmessages: + msg63315
2008-03-06 06:59:40georg.brandlsetnosy: + georg.brandl
messages: + msg63306
2008-03-05 21:55:21gpolosetfiles: + setitimer_getitimer_wrapper_rename.diff
messages: + msg63297
2008-03-05 21:43:24gpolosetfiles: + setitimer_getitimer_wrapper.diff
keywords: + patch
messages: + msg63295
2008-03-05 20:36:13loewissetnosy: + loewis
messages: + msg63293
2008-03-05 15:00:03facundobatistasetfiles: - py-itimer-0.1.1.tar.gz
2008-03-05 14:46:16gpolosetfiles: + py-itimer-0.1.2.tar.gz
messages: + msg63278
2008-03-05 14:42:12gpolocreate