log4perl-devel

[log4perl-devel] Util::module_available

[Eric W. <scr...@gm...>]

Hi

(sorry, I'm not on the list, please cc replies)

I've run into a bug in Log::Log4perl::Util::module_available when 
running in a par environment.  This may be an issue with PAR, but I 
have to guess that most @INC subroutine hooks are not expecting to be 
called directly.

The problem comes up in Log::Log4perl::Layout::PatternLayout (though I'm 
sure it could happen elsewhere in different circumstances.)  Basically, 
the module_available call returning true upsets the delicate balance of 
things (I'm guessing because the @INC hook sets an entry in %INC, which 
stops any further requires from actually loading the code.)

    if(Log::Log4perl::Util::module_available("Sys::Hostname")) {
        require Sys::Hostname;
        $HOSTNAME = Sys::Hostname::hostname();
    }

I'm wondering if there's some reason that cannot simply be an eval 
{ require Sys::Hostname}, (possibly checking that the error is m/^Can't 
locate Sys\/Hostname\.pm/ rather than some compile error.)

Thanks,
Eric
-- 
"Insert random misquote here"
---------------------------------------------------
    http://scratchcomputing.com
---------------------------------------------------

Re: [log4perl-devel] Util::module_available

[Mike S. <m...@pe...>]

On Tue, 23 Jan 2007, Eric Wilhelm wrote:

> I've run into a bug in Log::Log4perl::Util::module_available when
> running in a par environment.  This may be an issue with PAR, but I
> have to guess that most @INC subroutine hooks are not expecting to be
> called directly.

Hi Eric,

to work around issues with PAR, this FAQ shows a couple of tricks:

    http://log4perl.sourceforge.net/releases/Log-Log4perl/docs/html/Log/Log4perl/FAQ.html#9ea64

> The problem comes up in Log::Log4perl::Layout::PatternLayout (though I'm
> sure it could happen elsewhere in different circumstances.)  Basically,
> the module_available call returning true upsets the delicate balance of
> things (I'm guessing because the @INC hook sets an entry in %INC, which
> stops any further requires from actually loading the code.)
>
>     if(Log::Log4perl::Util::module_available("Sys::Hostname")) {
>         require Sys::Hostname;
>         $HOSTNAME = Sys::Hostname::hostname();
>     }
>
> I'm wondering if there's some reason that cannot simply be an eval
> { require Sys::Hostname}, (possibly checking that the error is m/^Can't
> locate Sys\/Hostname\.pm/ rather than some compile error.)

Not quite sure I understand the problem ... what's the error message
you're getting?

-- Mike

Mike Schilli
m...@pe...

>
> Thanks,
> Eric
> --
> "Insert random misquote here"
> ---------------------------------------------------
>     http://scratchcomputing.com
> ---------------------------------------------------
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share your
> opinions on IT & business topics through brief surveys - and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> _______________________________________________
> log4perl-devel mailing list
> log...@li...
> https://lists.sourceforge.net/lists/listinfo/log4perl-devel
>

Re: [log4perl-devel] Util::module_available

[Eric W. <scr...@gm...>]

# from Mike Schilli
# on Sunday 28 January 2007 07:30 pm:

>On Fri, 26 Jan 2007, Eric Wilhelm wrote:
>> The issue is that the code never gets compiled.  The @INC hook
>> sets[1] $INC{"foo/bar.pm"}, so any subsequent require()'s don't try
>> to load it. The errors will be equivalent to something like:
>>
>>   mkdir -p foo/Time
>>   echo "1;" > foo/Time/HiRes.pm
>>   perl -Ifoo -e 'use Time::HiRes';

Sorry, that last line should have read

  perl -Ifoo -e 'use Log::Log4perl'

Which should get you a huge pile of errors starting with:

  Undefined subroutine &Time::HiRes::gettimeofday called   
  at /usr/.../Log/Log4perl/Layout/PatternLayout.pm
  line 34.

Of course, nothing would work in this circumstance, but exactly the same 
thing is happening with your module_available() call in the presence of 
an @INC hook.

>> Because the @INC hook only returns a filehandle, to fully
>> re-implement require (which I don't suggest), you must
>> eval("$string") the contents of that.
>
>Can you send some perl code that shows the problem? I still don't
> understand what the problem is, especially where the @INC hook comes
> from.

Please see the attached demo.  The @INC hook in par is basically 
_find_par_internals(), which you can read here:

  http://search.cpan.org/src/SMUELLER/PAR-0.972/lib/PAR.pm

If you're still not seeing what's wrong with calling those coderefs and 
not eval($string)'ing the result, please read 'perldoc -f require' 
again.  "Note that these hooks are also permitted to set the %INC entry 
corresponding to the files they have loaded."

>> My question is:  why not eval {require Foo} ?
>Try this:
>    $SIG{__DIE__} = sub { print STDERR "Die! What?\n"; exit 0 };

I can understand the concern, but if that is the only reason, then I 
have to say it is bad trade-off to break a documented perl feature to 
allow for improper usage of a discouraged one.

That is a poorly written die hook which didn't follow the documentation.  
See perlfunc/die.  Any well-behaved SIG{__DIE__} should have:

  die @_ if $^S; # get out if we're in an eval

Though, you could always put local $SIG{__DIE__} in the eval if you're 
feeling lenient.  If you rework module_available() to use eval, that 
would be a good place for it.

Note, the attached (academic) example will demonstrate the issue.  
Calling it with no arguments means the first call to the hook is from 
module_available(), with a command-line argument, we'll require 
Sys::Hostname ourselves and module_available() will be happy.

Also attached is a patch which implements my proposed fix.

As I said, you could check whether $@ =~ /Can't locate $mod_path/, but 
that's only if you think compilation errors should be propagated (hint: 
put another curly around the "local $SIG{DIE}; eval {...}" chunk before 
you throw an error so it propagates correctly.)

Hopefully, the break_log4perl.pl attachment can be easily adapted to 
your test suite.  Please let me know if there's anything else I can do 
to help get this fixed.

Thanks,
Eric
-- 
"Left to themselves, things tend to go from bad to worse."
--Murphy's Corollary
---------------------------------------------------
    http://scratchcomputing.com
---------------------------------------------------