Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Perl: porters

[perl #53962] bug in Time::HiRes 5.11

 

 

Perl porters RSS feed   Index | Next | Previous | View Threaded


perlbug-followup at perl

May 10, 2008, 12:04 PM

Post #1 of 4 (117 views)
Permalink
[perl #53962] bug in Time::HiRes 5.11

# New Ticket Created by dk[at]tetsuo.karasik.eu.org
# Please include the string: [perl #53962]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=53962 >



This is a bug report for perl from dmitry[at]karasik.eu.org
generated with the help of perlbug 1.36 running under perl 5.11.0.


-----------------------------------------------------------------
[Please enter your report here]

Hello,

There's apparently bad behavior in Time::HiRes alarm code, in 5.11.0 and partly
in 5.10.0 . I hope that I'm wrong, but if I ain't, this seems to me a good
candidate to the daily WTF, because that's how one can accurately describe my
feeling when I saw the source :) The problem is simple:

use Time::HiRes qw(alarm);
my $tick;
$SIG{ALRM} = sub { $tick++ };
alarm(0.5);
sleep(1);
print "caught signal: ", ( $tick ? "ok" : "not ok"), "\n";

I was about to write the long story how many really strange conditions I found
crammed in these two small functions, but instead I'll just send the patch. I
don't have access to perl's source repositary to see the comments behind the
changes, but I see that beginning from 5.10 the strange changes began to
accumulate. I beg anyone interested to just have a look at the state what
ualarm() and alarm() became, to see if you can explain the logic to me.

Anyway, the patch (hopefully) restores the sanity back. The diff is for today's
(10 May) code I've pulled from git://utsl.gen.nz/perl-preview .

--- HiRes.xs.0 2008-05-10 20:02:23.000000000 +0200
+++ HiRes.xs 2008-05-10 20:42:27.000000000 +0200
@@ -925,7 +925,6 @@
CODE:
if (useconds < 0 || uinterval < 0)
croak("Time::HiRes::ualarm(%d, %d): negative time not invented yet", useconds, uinterval);
- if (useconds >= IV_1E6 || uinterval >= IV_1E6)
#if defined(HAS_SETITIMER) && defined(ITIMER_REAL)
{
struct itimerval itv;
@@ -936,11 +935,8 @@
}
}
#else
- croak("Time::HiRes::ualarm(%d, %d): useconds or uinterval equal to or more than %"IVdf, useconds, uinterval, IV_1E6);
-#endif
- else
RETVAL = ualarm(useconds, uinterval);
-
+#endif
OUTPUT:
RETVAL

@@ -954,7 +950,6 @@
{
IV useconds = IV_1E6 * seconds;
IV uinterval = IV_1E6 * interval;
- if (seconds >= IV_1E6 || interval >= IV_1E6)
#if defined(HAS_SETITIMER) && defined(ITIMER_REAL)
{
struct itimerval itv;
@@ -965,8 +960,7 @@
}
}
#else
- RETVAL = (NV)ualarm((IV)(seconds * IV_1E6),
- (IV)(interval * IV_1E6)) / NV_1E6;
+ RETVAL = ((NV) ualarm( useconds, uinterval)) / NV_1E6;
#endif
}


[Please do not change anything below this line]
-----------------------------------------------------------------
---
Flags:
category=core
severity=medium
---
Site configuration information for perl 5.11.0:

Configured by dk at Sat May 10 19:27:14 CEST 2008.

Summary of my perl5 (revision 5 version 11 subversion 0) configuration:
Platform:
osname=freebsd, osvers=6.2-stable, archname=i386-freebsd
uname='freebsd aguirre.karasik.eu.org 6.2-stable freebsd 6.2-stable #10: tue sep 11 19:36:55 cest 2007 root[at]aguirre.karasik.eu.org:usrobjusrsrcsysaguirre i386 '
config_args=''
hint=recommended, useposix=true, d_sigaction=define
useithreads=undef, usemultiplicity=undef
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=undef, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include',
optimize='-O',
cppflags='-DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -I/usr/local/include'
ccversion='', gccversion='3.4.6 [FreeBSD] 20060305', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=4, prototype=define
Linker and Libraries:
ld='cc', ldflags ='-Wl,-E -L/usr/local/lib'
libpth=/usr/lib /usr/local/lib
libs=-lgdbm -lm -lcrypt -lutil -lc
perllibs=-lm -lcrypt -lutil -lc
libc=, so=so, useshrplib=false, libperl=libperl.a
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' '
cccdlflags='-DPIC -fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
DEVEL

---
@INC for perl 5.11.0:
/usr/local/lib/perl5/5.11.0/i386-freebsd
/usr/local/lib/perl5/5.11.0
/usr/local/lib/perl5/site_perl/5.11.0/i386-freebsd
/usr/local/lib/perl5/site_perl/5.11.0
/usr/local/lib/perl5/site_perl
.

---
Environment for perl 5.11.0:
HOME=/home/dk
LANG=ru_RU.KOI8-R
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin:/usr/X11R6/bin:/home/dk/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/X11R6/bin:/usr/local/site/bin:/usr/local/site/sbin:/home/dk/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/X11R6/bin:/usr/local/site/bin:/usr/local/site/sbin:/home/dk/bin
PERL_BADLANG (unset)
SHELL=/usr/local/bin/zsh


shouldbedomo at mac

May 11, 2008, 6:32 AM

Post #2 of 4 (106 views)
Permalink
Re: [perl #53962] bug in Time::HiRes 5.11 [In reply to]

On 2008–05–10, at 21:04, dk[at]tetsuo.karasik.eu.org (via RT) wrote:

> I don't have access to perl's source repositary


The file, with line-by-line blame, should be visible to you at <http://public.activestate.com/cgi-bin/perlbrowse/b/ext/Time/HiRes/HiRes.xs
>. On brief examination, the check-in comments seem to do a good job
of saying why particular changes were made. As a general observation,
Time::HiRes has been frequently and diligently patched to accommodate
a seemingly never-ending stream of weirdness in how systems deal with
high-resolution time measurement. Consequently, the code is pretty ugly.
--
Dominic Dunlop


dmitry at karasik

May 11, 2008, 9:50 AM

Post #3 of 4 (101 views)
Permalink
Re: [perl #53962] bug in Time::HiRes 5.11 [In reply to]

Hi Dominic!

On 11 ΝΑΚ 08 at 15:32, "Dominic" (Dominic Dunlop) wrote:

Dominic> On 2008–05–10, at 21:04, dk[at]tetsuo.karasik.eu.org (via RT) wrote:
>> I don't have access to perl's source repositary
Dominic> The file, with line-by-line blame, should be visible to you at
Dominic> <http://public.activestate.com/cgi-bin/perlbrowse/b/ext/Time/HiRes/HiRes.xs
>> . On brief examination, the check-in comments seem to do a good job
Dominic> of saying why particular changes were made.

Ah, excellent. That did it - so there indeed was an explanation why timeouts
less than one second were implemented using setitimer(), and otherwise
using ualarm(). I've reworked the patch, so the time limit check stays, but
only for ualarm() case, and doesn't mix with the setitimer() logic.

--- HiRes.xs.0 2008-05-10 20:02:23.000000000 +0200
+++ HiRes.xs 2008-05-11 18:44:53.000000000 +0200
@@ -925,7 +925,6 @@
CODE:
if (useconds < 0 || uinterval < 0)
croak("Time::HiRes::ualarm(%d, %d): negative time not invented yet", useconds, uinterval);
- if (useconds >= IV_1E6 || uinterval >= IV_1E6)
#if defined(HAS_SETITIMER) && defined(ITIMER_REAL)
{
struct itimerval itv;
@@ -936,10 +935,10 @@
}
}
#else
+ if (useconds >= IV_1E6 || uinterval >= IV_1E6)
croak("Time::HiRes::ualarm(%d, %d): useconds or uinterval equal to or more than %"IVdf, useconds, uinterval, IV_1E6);
+ RETVAL = ualarm(useconds, uinterval);
#endif
- else
- RETVAL = ualarm(useconds, uinterval);

OUTPUT:
RETVAL
@@ -954,7 +953,6 @@
{
IV useconds = IV_1E6 * seconds;
IV uinterval = IV_1E6 * interval;
- if (seconds >= IV_1E6 || interval >= IV_1E6)
#if defined(HAS_SETITIMER) && defined(ITIMER_REAL)
{
struct itimerval itv;
@@ -965,8 +963,9 @@
}
}
#else
- RETVAL = (NV)ualarm((IV)(seconds * IV_1E6),
- (IV)(interval * IV_1E6)) / NV_1E6;
+ if (useconds >= IV_1E6 || uinterval >= IV_1E6)
+ croak("Time::HiRes::alarm(%d, %d): seconds or interval equal to or more than 1.0 ", useconds, uinterval, IV_1E6);
+ RETVAL = (NV)ualarm( useconds, uinterval) / NV_1E6;
#endif
}



--
Sincerely,
Dmitry Karasik


rurban at x-ray

May 12, 2008, 6:07 AM

Post #4 of 4 (90 views)
Permalink
Re: [perl #53962] bug in Time::HiRes 5.11 [In reply to]

Dmitry Karasik schrieb:
> On 11 ΝΑΚ 08 at 15:32, "Dominic" (Dominic Dunlop) wrote:
>
> Dominic> On 2008–05–10, at 21:04, dk[at]tetsuo.karasik.eu.org (via RT) wrote:
> >> I don't have access to perl's source repositary
> Dominic> The file, with line-by-line blame, should be visible to you at
> Dominic> <http://public.activestate.com/cgi-bin/perlbrowse/b/ext/Time/HiRes/HiRes.xs
> >> . On brief examination, the check-in comments seem to do a good job
> Dominic> of saying why particular changes were made.
>
> Ah, excellent. That did it - so there indeed was an explanation why timeouts
> less than one second were implemented using setitimer(), and otherwise
> using ualarm(). I've reworked the patch, so the time limit check stays, but
> only for ualarm() case, and doesn't mix with the setitimer() logic.

That patch didn't fix my ongoing cygwin blead failures.

ext/Time/HiRes/t/HiRes........................................FAILED--non-zero
wait status: 15

same error before and after.

> --- HiRes.xs.0 2008-05-10 20:02:23.000000000 +0200
> +++ HiRes.xs 2008-05-11 18:44:53.000000000 +0200
> @@ -925,7 +925,6 @@
> CODE:
> if (useconds < 0 || uinterval < 0)
> croak("Time::HiRes::ualarm(%d, %d): negative time not invented yet", useconds, uinterval);
> - if (useconds >= IV_1E6 || uinterval >= IV_1E6)
> #if defined(HAS_SETITIMER) && defined(ITIMER_REAL)
> {
> struct itimerval itv;
> @@ -936,10 +935,10 @@
> }
> }
> #else
> + if (useconds >= IV_1E6 || uinterval >= IV_1E6)
> croak("Time::HiRes::ualarm(%d, %d): useconds or uinterval equal to or more than %"IVdf, useconds, uinterval, IV_1E6);
> + RETVAL = ualarm(useconds, uinterval);
> #endif
> - else
> - RETVAL = ualarm(useconds, uinterval);
>
> OUTPUT:
> RETVAL
> @@ -954,7 +953,6 @@
> {
> IV useconds = IV_1E6 * seconds;
> IV uinterval = IV_1E6 * interval;
> - if (seconds >= IV_1E6 || interval >= IV_1E6)
> #if defined(HAS_SETITIMER) && defined(ITIMER_REAL)
> {
> struct itimerval itv;
> @@ -965,8 +963,9 @@
> }
> }
> #else
> - RETVAL = (NV)ualarm((IV)(seconds * IV_1E6),
> - (IV)(interval * IV_1E6)) / NV_1E6;
> + if (useconds >= IV_1E6 || uinterval >= IV_1E6)
> + croak("Time::HiRes::alarm(%d, %d): seconds or interval equal to or more than 1.0 ", useconds, uinterval, IV_1E6);
> + RETVAL = (NV)ualarm( useconds, uinterval) / NV_1E6;
> #endif
> }


--
Reini Urban
http://phpwiki.org/ http://murbreak.at/

Perl porters RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact lists@gossamer-threads.com
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.