misplacement of alarm(0) leaks alarms

[moregan]

PPI::Document contains multiple instances of an alarm not being turned off in the normal case of lex_file returning a document:

                        if ( $timeout ) {
                                eval {
                                        local $SIG{ALRM} = sub { die "alarm\n" };
                                        alarm( $timeout );
                                        my $document = PPI::Lexer->lex_file( $source );
                                        return $class->_setattr( $document, %attr ) if $document;
                                        alarm( 0 );
                                };
                        } else {
                                my $document = PPI::Lexer->lex_file( $source );
                                return $class->_setattr( $document, %attr ) if $document;
                        }

Doesn't the return ahead of alarm( 0 ) mean that if the process lives long enough the then-active ALRM handler will be called? I rarely see/use alarm(), so any light to shed here, @adamkennedy ?

[adamkennedy]

Because I spent most of my time on Windows, where alarm doesn't work, I don't completely trust my code in this regard.

Adam On Nov 27, 2014 6:36 PM, "moregan" notifications@github.com wrote:

PPI::Document contains multiple instances of an alarm not being turned off in the normal case of lex_file returning a document:

                    if ( $timeout ) {
                            eval {
                                    local $SIG{ALRM} = sub { die "alarm\n" };
                                    alarm( $timeout );
                                    my $document = PPI::Lexer->lex_file( $source );
                                    return $class->_setattr( $document, %attr ) if $document;
                                    alarm( 0 );
                            };
                    } else {
                            my $document = PPI::Lexer->lex_file( $source );
                            return $class->_setattr( $document, %attr ) if $document;
                    }

Doesn't the return ahead of alarm( 0 ) mean that if the process lives long enough the then-active ALRM handler will be called? I rarely see/use alarm(), so any light to shed here, @adamkennedy https://github.com/adamkennedy ?

— Reply to this email directly or view it on GitHub https://github.com/adamkennedy/PPI/issues/140.

[wchristian]

perl-help opines that this is indeed a bug.

[moregan]

See #143 about the fate of Exception::ParserTimeout

[moregan]

The user-facing HAVE_ALARM and PPI::Exception::ParserTimeout are both unused on CPAN, so the timeout feature is probably safe to remove altogether