Skip to content
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.

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

update of perlfunc alarm() synopsis #15902

Open
p5pRT opened this issue Mar 2, 2017 · 10 comments
Open

update of perlfunc alarm() synopsis #15902

p5pRT opened this issue Mar 2, 2017 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 2, 2017

Migrated from rt.perl.org#130902 (status was 'open')

Searchable as RT130902$

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2017

From jozef.kutej@geizhals.at

Hi,

attached is suggestion-patch for alarm() synopsis.

The current version by "local $SIG{ALRM}" implies that the previous SIGALARM functionality is restored. But it fact if will never by fired because of the timer it self not restored. Proposed version should point out this fact and demonstrate the possible usage of alarm() return value too.

Best regards
Jozef

--
Mit freundlichen Grüßen

Jozef Kutej
Software Development

Geizhals (R) - Preisvergleich
Preisvergleich Internet Services AG
Obere Donaustraße 63/2
A-1020 Wien
Tel​: +43 1 5811609/62
Fax​: +43 1 5811609/55

http​://www.geizhals.at | http​://www.geizhals.de | http​://www.geizhals.eu
http​://www.facebook.com/geizhals => Geizhals auf Facebook!
http​://twitter.com/geizhals => Geizhals auf Twitter!
http​://blog.geizhals.at => Der Geizhals-Blog!
http​://unternehmen.geizhals.at/about/de/apps/ => Die Geizhals Mobile-App

Handelsgericht Wien | FN 197241K | Firmensitz Wien

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2017

From jozef.kutej@geizhals.at

alarm-synopsis.patch
diff --git a/cpan/Pod-Perldoc/corpus/perlfunc.pod b/cpan/Pod-Perldoc/corpus/perlfunc.pod
index 604123daf8..d06fb50e31 100644
--- a/cpan/Pod-Perldoc/corpus/perlfunc.pod
+++ b/cpan/Pod-Perldoc/corpus/perlfunc.pod
@@ -497,11 +497,12 @@ fail with C<$!> set to C<EINTR> because Perl sets up signal handlers to
 restart system calls on some systems.  Using C<eval>/C<die> always works,
 modulo the caveats given in L<perlipc/"Signals">.
 
+    my $timeout = 10;
     eval {
-        local $SIG{ALRM} = sub { die "alarm\n" }; # NB: \n required
-        alarm $timeout;
-        $nread = sysread SOCKET, $buffer, $size;
-        alarm 0;
+        my $previous_timeout = alarm $timeout;
+        local $SIG{ALRM} = sub { alarm $previous_timeout; die "alarm\n" }; # NB: \n required
+        my $nread = sysread $socket, $buffer, $size;
+        alarm $previous_timeout;
     };
     if ($@) {
         die unless $@ eq "alarm\n";   # propagate unexpected errors
diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod
index 3854acf6e6..d7d5922678 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@@ -732,11 +732,12 @@ to restart system calls on some systems.  Using
 L<C<eval>|/eval EXPR>/L<C<die>|/die LIST> always works, modulo the
 caveats given in L<perlipc/"Signals">.
 
+    my $timeout = 10;
     eval {
-        local $SIG{ALRM} = sub { die "alarm\n" }; # NB: \n required
-        alarm $timeout;
+        my $previous_timeout = alarm $timeout;
+        local $SIG{ALRM} = sub { alarm $previous_timeout; die "alarm\n" }; # NB: \n required
         my $nread = sysread $socket, $buffer, $size;
-        alarm 0;
+        alarm $previous_timeout;
     };
     if ($@) {
         die unless $@ eq "alarm\n";   # propagate unexpected errors

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2017

From @Leont

On Thu, Mar 2, 2017 at 2​:54 PM, Jozef Kutej <perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Jozef Kutej
# Please include the string​: [perl #130902]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=130902 >

Hi,

attached is suggestion-patch for alarm() synopsis.

The current version by "local $SIG{ALRM}" implies that the previous SIGALARM functionality is restored. But it fact if will never by fired because of the timer it self not restored. Proposed version should point out this fact and demonstrate the possible usage of alarm() return value too.

Best regards
Jozef

Why would you want to set the alarm to some previous value it had at
some previous point in time? I can't think of any situation where this
is correct.I f your code can't be sure it's the only thing using
alarm/SIGALRM, you should be using something else in the first place
(for example select, an event loop, or maybe real-time signals).

Leon

@p5pRT
Copy link
Author

p5pRT commented Mar 2, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2017

From @jkeenan

On Thu, 02 Mar 2017 13​:54​:25 GMT, jozef.kutej@​geizhals.at wrote​:

Hi,

attached is suggestion-patch for alarm() synopsis.

The current version by "local $SIG{ALRM}" implies that the previous
SIGALARM functionality is restored. But it fact if will never by fired
because of the timer it self not restored. Proposed version should
point out this fact and demonstrate the possible usage of alarm()
return value too.

Best regards
Jozef

The patch provided calls for modifications to pod/perlfunc.pod and to a file in the Pod-Perldoc distribution. The latter is maintained upstream on CPAN. I am cc-ing the maintainer of that distribution for comments before proceeding.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jun 1, 2017

From @demerphq

On 2 March 2017 at 18​:16, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Thu, Mar 2, 2017 at 2​:54 PM, Jozef Kutej <perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Jozef Kutej
# Please include the string​: [perl #130902]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=130902 >

Hi,

attached is suggestion-patch for alarm() synopsis.

The current version by "local $SIG{ALRM}" implies that the previous SIGALARM functionality is restored. But it fact if will never by fired because of the timer it self not restored. Proposed version should point out this fact and demonstrate the possible usage of alarm() return value too.

Best regards
Jozef

Why would you want to set the alarm to some previous value it had at
some previous point in time? I can't think of any situation where this
is correct.I f your code can't be sure it's the only thing using
alarm/SIGALRM, you should be using something else in the first place
(for example select, an event loop, or maybe real-time signals).

There is only one alarm timer per process.

A well behaved user of alarm will capture the return from their own
call to alarm() and then manage things so that the previous alarm, if
any, is restored, minus any elapsed time afterwards.

See Time​::Out on cpan for an example of this pattern. Something like this​:

$remaining= alarm(0);
{
local $SIG{ALARM} = sub { .... };
alarm($time);
eval { do_stuff(); 1 } or .....;
}
alarm($remaining - $elapsed_time) if $remaining;

But more complicated. If you check the code in Time​::Out it is not
easy to do right, but it is possible, and I think probably the OP is
right that we should document how to do it right.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Jun 2, 2017

From @Leont

On Thu, Jun 1, 2017 at 5​:06 PM, demerphq <demerphq@​gmail.com> wrote​:

On 2 March 2017 at 18​:16, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Thu, Mar 2, 2017 at 2​:54 PM, Jozef Kutej <perlbug-followup@​perl.org>
wrote​:

# New Ticket Created by Jozef Kutej
# Please include the string​: [perl #130902]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=130902 >

Hi,

attached is suggestion-patch for alarm() synopsis.

The current version by "local $SIG{ALRM}" implies that the previous
SIGALARM functionality is restored. But it fact if will never by fired
because of the timer it self not restored. Proposed version should point
out this fact and demonstrate the possible usage of alarm() return value
too.

Best regards
Jozef

Why would you want to set the alarm to some previous value it had at
some previous point in time? I can't think of any situation where this
is correct.I f your code can't be sure it's the only thing using
alarm/SIGALRM, you should be using something else in the first place
(for example select, an event loop, or maybe real-time signals).

There is only one alarm timer per process.

Yes I fully realize there can only be one alarm timer per process, I didn't
write POSIX​::RT​::Timer for nothing ;-)

A well behaved user of alarm will capture the return from their own

call to alarm() and then manage things so that the previous alarm, if
any, is restored, minus any elapsed time afterwards.

See Time​::Out on cpan for an example of this pattern. Something like this​:

$remaining= alarm(0);
{
local $SIG{ALARM} = sub { .... };
alarm($time);
eval { do_stuff(); 1 } or .....;
}
alarm($remaining - $elapsed_time) if $remaining;

But more complicated. If you check the code in Time​::Out it is not
easy to do right, but it is possible, and I think probably the OP is
right that we should document how to do it right.

I don't quite think Time​::Out is entirely correct either (e.g. it's not
thread-safe, sensitive to wallclock-moving, doesn't interact well with
blocking XS calls, may leave zombies and other leaks, may cause race
conditions…). The only two cases where I'd consider using alarm are​:
1. I want my script to die if it's taking too long (this would be top-level
and usually wouldn't need a signal handler). I actually do this (with a
wide time margin) in a lot of tests that could hang.
2. I need to deal with some unfortunate XS code that doesn't give my any
other timeout options. such as DBD​::Oracle's connect (this would usually
need setting an unsafe signal handler to be effective).

In all other cases there are better approaches to this; the example is
clearly such a case. I think we're better of explaining that instead.

Leon

@p5pRT
Copy link
Author

p5pRT commented Jun 2, 2017

From @demerphq

On 2 June 2017 at 02​:55, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Thu, Jun 1, 2017 at 5​:06 PM, demerphq <demerphq@​gmail.com> wrote​:

On 2 March 2017 at 18​:16, Leon Timmermans <fawaka@​gmail.com> wrote​:

On Thu, Mar 2, 2017 at 2​:54 PM, Jozef Kutej <perlbug-followup@​perl.org>
wrote​:

# New Ticket Created by Jozef Kutej
# Please include the string​: [perl #130902]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=130902 >

Hi,

attached is suggestion-patch for alarm() synopsis.

The current version by "local $SIG{ALRM}" implies that the previous
SIGALARM functionality is restored. But it fact if will never by fired
because of the timer it self not restored. Proposed version should point out
this fact and demonstrate the possible usage of alarm() return value too.

Best regards
Jozef

Why would you want to set the alarm to some previous value it had at
some previous point in time? I can't think of any situation where this
is correct.I f your code can't be sure it's the only thing using
alarm/SIGALRM, you should be using something else in the first place
(for example select, an event loop, or maybe real-time signals).

There is only one alarm timer per process.

Yes I fully realize there can only be one alarm timer per process, I didn't
write POSIX​::RT​::Timer for nothing ;-)

A well behaved user of alarm will capture the return from their own
call to alarm() and then manage things so that the previous alarm, if
any, is restored, minus any elapsed time afterwards.

See Time​::Out on cpan for an example of this pattern. Something like this​:

$remaining= alarm(0);
{
local $SIG{ALARM} = sub { .... };
alarm($time);
eval { do_stuff(); 1 } or .....;
}
alarm($remaining - $elapsed_time) if $remaining;

But more complicated. If you check the code in Time​::Out it is not
easy to do right, but it is possible, and I think probably the OP is
right that we should document how to do it right.

I don't quite think Time​::Out is entirely correct either (e.g. it's not
thread-safe,

I don't think people who use alarm use threads. None of our work
servers are built for threading certainly.

sensitive to wallclock-moving, doesn't interact well with
blocking XS calls, may leave zombies and other leaks, may cause race
conditions…). The only two cases where I'd consider using alarm are​:

Well its still better than nothing.

1. I want my script to die if it's taking too long (this would be top-level
and usually wouldn't need a signal handler). I actually do this (with a wide
time margin) in a lot of tests that could hang.

For that you are probably better using a watchdog type functionality.

2. I need to deal with some unfortunate XS code that doesn't give my any
other timeout options. such as DBD​::Oracle's connect (this would usually
need setting an unsafe signal handler to be effective).

Yeah well exactly.

We use alarm() to timeout our web-requests, and we use alarm() to time
out connect requests.

So we end up using the Time​::Out pattern. (Except I hacked it to
resolve various issues in its API.)

I personally think that documenting all of this is important.

Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Jun 2, 2017

From @Leont

On Thu, Jun 1, 2017 at 4​:24 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Thu, 02 Mar 2017 13​:54​:25 GMT, jozef.kutej@​geizhals.at wrote​:

Hi,

attached is suggestion-patch for alarm() synopsis.

The current version by "local $SIG{ALRM}" implies that the previous
SIGALARM functionality is restored. But it fact if will never by fired
because of the timer it self not restored. Proposed version should
point out this fact and demonstrate the possible usage of alarm()
return value too.

Best regards
Jozef

The patch provided calls for modifications to pod/perlfunc.pod and to a
file in the Pod-Perldoc distribution. The latter is maintained upstream on
CPAN. I am cc-ing the maintainer of that distribution for comments before
proceeding.

Thank you very much.

The latter is a copy of the former for testing purposes.

Leon

@p5pRT
Copy link
Author

p5pRT commented Jun 25, 2017

From @mrallen1

On Thu, 01 Jun 2017 07​:24​:14 -0700, jkeenan wrote​:

The patch provided calls for modifications to pod/perlfunc.pod and to
a file in the Pod-Perldoc distribution. The latter is maintained
upstream on CPAN. I am cc-ing the maintainer of that distribution for
comments before proceeding.

As Leon noted before, the file in Pod-Perldoc is a copy for testing purposes only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants