X Tutup
Skip to content

bpo-30807: signal.setitimer() may disable the timer by mistake #2493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pitrou merged 2 commits into from
Jun 30, 2017
Merged

bpo-30807: signal.setitimer() may disable the timer by mistake #2493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9234d63
bpo-30807: signal.setitimer() may disable the timer by mistake
pitrou Jun 29, 2017
05e6bb3
Add NEWS blurb
pitrou Jun 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Lib/test/test_signal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,15 @@ def test_itimer_prof(self):
# and the handler should have been called
self.assertEqual(self.hndl_called, True)

def test_setitimer_tiny(self):
# bpo-30807: C setitimer() takes a microsecond-resolution interval.
# Check that float -> timeval conversion doesn't round
# the interval down to zero, which would disable the timer.
self.itimer = signal.ITIMER_REAL
signal.setitimer(self.itimer, 1e-6)
Copy link
Copy Markdown
Member

@vstinner vstinner Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to test 1e-9 to make sure that it's smaller than 1 us, since 1e-6 is not an exact floating number number.

>>> (1e-6).hex()
'0x1.0c6f7a0b5ed8dp-20'

Copy link
Copy Markdown
Member Author

@pitrou pitrou Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't understand. 1e-6 is the minimum advertised resolution for setitimer, that's why it's supposed to work.

Copy link
Copy Markdown
Member Author

@pitrou pitrou Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, your suggestion would make the test fail reliably without the patch. I'm on the (signal) fence.

time.sleep(1)
self.assertEqual(self.hndl_called, True)


class PendingSignalsTests(unittest.TestCase):
"""
Expand Down
6 changes: 6 additions & 0 deletions Misc/NEWS.d/next/Library/2017-06-29-22-04-44.bpo-30807.sLtjY-.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
signal.setitimer() may disable the timer when passed a tiny value.

Tiny values (such as 1e-6) are valid non-zero values for setitimer(), which
Copy link
Copy Markdown
Member

@vstinner vstinner Jun 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, I'm not sure about your Git-commit-message-like formatting. I may be needed to agree on a style on python-dev.

Copy link
Copy Markdown
Member Author

@pitrou pitrou Jun 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really care about this :-)

is specified as taking microsecond-resolution intervals. However, on some
platform, our conversion routine could convert 1e-6 into a zero interval,
therefore disabling the timer instead of (re-)scheduling it.
4 changes: 4 additions & 0 deletions Modules/signalmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ timeval_from_double(double d, struct timeval *tv)
{
tv->tv_sec = floor(d);
tv->tv_usec = fmod(d, 1.0) * 1000000.0;
/* Don't disable the timer if the computation above rounds down to zero. */
if (d > 0.0 && tv->tv_sec == 0 && tv->tv_usec == 0) {
tv->tv_usec = 1;
}
}

Py_LOCAL_INLINE(double)
Expand Down
X Tutup